From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2a07:de40:b251:101:10:150:64:1]) by sourceware.org (Postfix) with ESMTPS id 9470D3858CD1 for ; Fri, 23 Feb 2024 16:51:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9470D3858CD1 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 9470D3858CD1 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a07:de40:b251:101:10:150:64:1 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1708707090; cv=none; b=hQ74heBUrXdcmBYhLCNLXiGOYE6pk4x8mlD2P95iGlvTYvy5Npga1N2IZ9HWcfjfbBtpXno6BP1ZrRwMK3y2OVcNmErEwQAa+jy+0nz3PQpBOm8dxM+mxVnqy7PMoSLrcOgaLFgL7FnjqcL6ahNljPBYFODAOz1ZVBUu19Wsa/8= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1708707090; c=relaxed/simple; bh=PwrFFbnlGqEZqiBzhzlxVj7OQXU07PP9jdMjc6L34gk=; h=DKIM-Signature:DKIM-Signature:DKIM-Signature:DKIM-Signature:From: To:Subject:Date:Message-Id:MIME-Version; b=K2mNrqPSbJp225iVbHyprlQnPuuF2/eBpFizbsIJSQBordLn6bD8JNYfiS8u7DOJnBdP1TWUPEt9gZaaVnEeXyNnNZXINlVxl5kyEJ3v7yE3ZI2XPAHPxcJBWkfh6VoqAqpTKRFmtxhIMxvzqPnA9iko93cbKPBf9E/6vFumcdc= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from imap1.dmz-prg2.suse.org (imap1.dmz-prg2.suse.org [10.150.64.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 5F3D521F16; Fri, 23 Feb 2024 16:51:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1708707087; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=8OMYK6wVAT0p0d8zBfgFjCZzyUnNqThFJS4u5Aj8hCQ=; b=DCtVwrQsnguqNxNaQQEzDPeYlo0q9VBwkqDgR2iaatVe3izMSmQCt5PEfazlMG+FvjQ0Ih DMNgEe1rYjVOHnDRg5rhZszS3/U0NXDwCcwH0mjAklqMW6TgBFZBdWMhK1GuMHgA239X6J 2DFVHg4kno/UUOZ+Mkk9eDDSjreut10= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1708707087; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=8OMYK6wVAT0p0d8zBfgFjCZzyUnNqThFJS4u5Aj8hCQ=; b=WhA1l4ZpVE131evevwjUE/6L/TlhgfvEqu4iL3WWUZe8E863EDDX8t5tsc429EIj8Eh9s/ 9vTM6y8gxaN81YDQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1708707087; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=8OMYK6wVAT0p0d8zBfgFjCZzyUnNqThFJS4u5Aj8hCQ=; b=DCtVwrQsnguqNxNaQQEzDPeYlo0q9VBwkqDgR2iaatVe3izMSmQCt5PEfazlMG+FvjQ0Ih DMNgEe1rYjVOHnDRg5rhZszS3/U0NXDwCcwH0mjAklqMW6TgBFZBdWMhK1GuMHgA239X6J 2DFVHg4kno/UUOZ+Mkk9eDDSjreut10= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1708707087; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=8OMYK6wVAT0p0d8zBfgFjCZzyUnNqThFJS4u5Aj8hCQ=; b=WhA1l4ZpVE131evevwjUE/6L/TlhgfvEqu4iL3WWUZe8E863EDDX8t5tsc429EIj8Eh9s/ 9vTM6y8gxaN81YDQ== Received: from imap1.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id 41DF013AC1; Fri, 23 Feb 2024 16:51:27 +0000 (UTC) Received: from dovecot-director2.suse.de ([2a07:de40:b281:106:10:150:64:167]) by imap1.dmz-prg2.suse.org with ESMTPSA id gEm5Dg/N2GUINwAAD6G6ig (envelope-from ); Fri, 23 Feb 2024 16:51:27 +0000 From: Tom de Vries To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [PATCH 2/2] [gdb/dap] Fix stray KeyboardInterrupt after cancel Date: Fri, 23 Feb 2024 17:51:28 +0100 Message-Id: <20240223165128.32211-2-tdevries@suse.de> X-Mailer: git-send-email 2.35.3 In-Reply-To: <20240223165128.32211-1-tdevries@suse.de> References: <20240223165128.32211-1-tdevries@suse.de> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Authentication-Results: smtp-out1.suse.de; none X-Spamd-Result: default: False [1.90 / 50.00]; ARC_NA(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; R_MISSING_CHARSET(2.50)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; MIME_GOOD(-0.10)[text/plain]; BROKEN_CONTENT_TYPE(1.50)[]; RCVD_COUNT_THREE(0.00)[3]; DKIM_SIGNED(0.00)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; RCPT_COUNT_TWO(0.00)[2]; MID_CONTAINS_FROM(1.00)[]; FUZZY_BLOCKED(0.00)[rspamd.com]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; RCVD_TLS_ALL(0.00)[]; BAYES_HAM(-3.00)[100.00%] X-Spam-Score: 1.90 X-Spam-Status: No, score=-12.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: When running test-case gdb.dap/pause.exp 100 times in a loop, it passes 100/100. But if we remove the two "sleep 0.2" from the test-case, we run into (copied from dap.log and edited for readability): ... Traceback (most recent call last): File "startup.py", line 251, in message def message(): KeyboardInterrupt Quit ... This happens as follows. CancellationHandler.cancel calls gdb.interrupt to cancel a request in flight. The idea is that this interrupt triggers while in fn here in message (a nested function of send_gdb_with_response): ... def message(): try: val = fn() result_q.put(val) except (Exception, KeyboardInterrupt) as e: result_q.put(e) ... but instead it triggers outside the try/except. Fix this by: - in CancellationHandler, adding an variable interruptable to track whether we're in the try/except, - in CancellationHandler.cancel, setting a variable cancel_pending instead of callling gdb.interrupt if not in the try/except, and - handling cancel_pending in a few places. This makes the test-case pass 100/100, also when adding the extra stressor of "taskset -c 0", which makes the fail more likely without the patch. I left the two "sleep 0.2" in the test-case, I didn't see a reason to remove those. Tested on aarch64-linux. PR dap/31275 Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31275 --- gdb/python/lib/gdb/dap/server.py | 42 ++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/gdb/python/lib/gdb/dap/server.py b/gdb/python/lib/gdb/dap/server.py index ecec41cc321..0b27d23da1e 100644 --- a/gdb/python/lib/gdb/dap/server.py +++ b/gdb/python/lib/gdb/dap/server.py @@ -62,6 +62,8 @@ class CancellationHandler: # The request currently being handled, or None. self.in_flight = None self.reqs = [] + self.interruptable = False + self.cancel_pending = False def starting(self, req): """Call at the start of the given request. @@ -87,7 +89,10 @@ class CancellationHandler: If the request has not yet been seen, the cancellation is queued.""" with self.lock: if req == self.in_flight: - gdb.interrupt() + if self.interruptable: + gdb.interrupt() + else: + self.cancel_pending = True else: # We don't actually ignore the request here, but in # the 'starting' method. This way we don't have to @@ -97,6 +102,29 @@ class CancellationHandler: # to try to check for this. heapq.heappush(self.reqs, req) + def is_cancel_pending(self): + """Is a cancellation pending. This can only be true when not in the + interruptable state.""" + with self.lock: + if not self.cancel_pending: + return False + self.cancel_pending = False + return True + + def enter_interruptable(self): + """Enter the interruptable state.""" + with self.lock: + if self.cancel_pending: + self.cancel_pending = False + return False + self.interruptable = True + return True + + def exit_interruptable(self): + """Exit the interruptable state.""" + with self.lock: + self.interruptable = False + class Server: """The DAP server class.""" @@ -159,6 +187,9 @@ class Server: result["success"] = False result["message"] = str(e) self.canceller.done(req) + if self.canceller.is_cancel_pending(): + result["success"] = False + result["message"] = "cancelled" return result # Read inferior output and sends OutputEvents to the client. It @@ -435,12 +466,19 @@ def send_gdb_with_response(fn): def message(): try: - val = fn() + if not _server.canceller.enter_interruptable(): + raise KeyboardInterrupt() + try: + val = fn() + finally: + _server.canceller.exit_interruptable() result_q.put(val) except (Exception, KeyboardInterrupt) as e: result_q.put(e) send_gdb(message) + if _server.canceller.is_cancel_pending(): + result_q.put(KeyboardInterrupt()) val = result_q.get() if isinstance(val, (Exception, KeyboardInterrupt)): raise val -- 2.35.3