From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-f66.google.com (mail-wm1-f66.google.com [209.85.128.66]) by sourceware.org (Postfix) with ESMTPS id BDD153858D37 for ; Mon, 6 Jul 2020 19:02:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org BDD153858D37 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=alves.ped@gmail.com Received: by mail-wm1-f66.google.com with SMTP id a6so185641wmm.0 for ; Mon, 06 Jul 2020 12:02:57 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references; bh=6KkznYew+oXlNP4kHzI+6Pg2Dek+Gzf6BWPG+JZ5bL8=; b=K+IanOnxJ8YNerUlowLQAQn8VXUT7ADMs96rzadqNXyyGlBZsbXT7wiU4bzXtiJs/4 NDC+NPoSuGKkeFSK/CB6HzwqVne6OQGV82G6bUX4Yi9DA4QHzD39CIhMY/+KMStRtKLi Ci1kLqvdV1qfVdT7yygJV/FkKXUrmk7CdL9lo3V+AZD9uvNrZObIyMQbvNBiHT82ce0l VggLbwCxg9pTpSAqZj1jl9jtI9LChFcM+QHhlmwkh497ggbdZ+7Zcuwb0KwmEWhzDt5M 5RLgZ11m5Z5OJmUDbwPC6piixtdR7SPBl0quMZZEOIc8Ak8eHKQb4HGaSaJ5MjTRDmF0 ADKQ== X-Gm-Message-State: AOAM530Ui7mhix2oqWLoP6ukdsr5hCMEY0mOPoNcWy05KmvqgLCX/guZ r8bzNBReq4xwTzt+jNpIYuIxJ4e6P7qzIQ== X-Google-Smtp-Source: ABdhPJwHHBL/CGgKA7Fv58LniBF1qq7Cuww9F+ZWFIWblwgoGUujH2g1ByfqzPzu9u/8hG5Tk182/w== X-Received: by 2002:a1c:f003:: with SMTP id a3mr576686wmb.119.1594062176594; Mon, 06 Jul 2020 12:02:56 -0700 (PDT) Received: from localhost ([2001:8a0:f91a:c400:8728:8fef:5b85:5934]) by smtp.gmail.com with ESMTPSA id x1sm25152870wrp.10.2020.07.06.12.02.55 for (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Mon, 06 Jul 2020 12:02:55 -0700 (PDT) From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH 1/7] Fix spurious unhandled remote %Stop notifications Date: Mon, 6 Jul 2020 20:02:46 +0100 Message-Id: <20200706190252.22552-2-pedro@palves.net> X-Mailer: git-send-email 2.14.5 In-Reply-To: <20200706190252.22552-1-pedro@palves.net> References: <20200706190252.22552-1-pedro@palves.net> X-Spam-Status: No, score=-8.7 required=5.0 tests=BAYES_00, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 06 Jul 2020 19:02:59 -0000 In non-stop mode, remote targets mark an async event source whose callback is supposed to result in calling remote_target::wait_ns to either process the event queue, or acknowledge an incoming %Stop notification. The callback in question is remote_async_inferior_event_handler, where we call inferior_event_handler, to end up in fetch_inferior_event -> target_wait -> remote_target::wait -> remote_target::wait_ns. A problem here however is that when debugging multiple targets, fetch_inferior_event can pull events out of any target picked at random, for event fairness. This means that when remote_async_inferior_event_handler returns, remote_target::wait may have not been called at all, and thus pending notifications may have not been acked. Because async event sources auto-clear, when remote_async_inferior_event_handler returns the async event handler is no longer marked, so the event loop won't automatically call remote_async_inferior_event_handler again to try to process the pending remote notifications/queue. The result is that stop events may end up not processed, e.g., "interrupt -a" seemingly not managing to stop all threads. Fix this by making remote_async_inferior_event_handler mark the event handler again before returning, if necessary. Maybe a better fix would be to make async event handlers not auto-clear themselves, make that the responsibility of the callback, so that the event loop would keep calling the callback automatically. Or, we could try making so that fetch_inferior_event would optionally handle events only for the target that it got passed down via parameter. However, I don't think now just before branching is the time to try to do any such change. gdb/ChangeLog: PR gdb/26199 * remote.c (remote_target::open_1): Pass remote target pointer as data to create_async_event_handler. (remote_async_inferior_event_handler): Mark async event handler before returning if the remote target still has either pending events or unacknowledged notifications. --- gdb/remote.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/gdb/remote.c b/gdb/remote.c index f7f99dc24f..59075cb09f 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -5605,7 +5605,7 @@ remote_target::open_1 (const char *name, int from_tty, int extended_p) /* Register extra event sources in the event loop. */ rs->remote_async_inferior_event_token - = create_async_event_handler (remote_async_inferior_event_handler, NULL); + = create_async_event_handler (remote_async_inferior_event_handler, remote); rs->notif_state = remote_notif_state_allocate (remote); /* Reset the target state; these things will be queried either by @@ -14164,6 +14164,19 @@ static void remote_async_inferior_event_handler (gdb_client_data data) { inferior_event_handler (INF_REG_EVENT); + + remote_target *remote = (remote_target *) data; + remote_state *rs = remote->get_remote_state (); + + /* inferior_event_handler may have consumed an event pending on the + infrun side without calling target_wait on the REMOTE target, or + may have pulled an event out of a different target. Keep trying + for this remote target as long it still has either pending events + or unacknowledged notifications. */ + + if (rs->notif_state->pending_event[notif_client_stop.id] != NULL + || !rs->stop_reply_queue.empty ()) + mark_async_event_handler (rs->remote_async_inferior_event_token); } int -- 2.14.5