From patchwork Mon Feb 26 11:58:25 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom de Vries X-Patchwork-Id: 86374 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 8B5BA3858C39 for ; Mon, 26 Feb 2024 11:59:10 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.223.130]) by sourceware.org (Postfix) with ESMTPS id 93A223858C55 for ; Mon, 26 Feb 2024 11:58:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 93A223858C55 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 93A223858C55 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=195.135.223.130 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1708948700; cv=none; b=YMSO7tGY5s1eBVQsSSQnehve3sYZ0+GexuAgeIVVncPcKamrP5oyZ76G3WBDtETiHfSSvIDO1KyNZyZgsB7zjAUWeBL/I39NvHcybQJL0Xn470o0zKTW6UXJpR+LTxnsb1jFWo0Z7Y8Yyqt7m2hWKreJdR5emgkquGi5BO0Fiqw= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1708948700; c=relaxed/simple; bh=/S4mCc1JFkWfM0nJn3AxaPoOD1/RR42pLN84fnie0yY=; h=DKIM-Signature:DKIM-Signature:DKIM-Signature:DKIM-Signature:From: To:Subject:Date:Message-Id:MIME-Version; b=ofXMuVBbStp2HQ1VpvtJkAvNAKSeWVDMXc0UgDVf/w5BQBOj6sSCDwj2/FRlieI3giEPag2+0XIK+3KhpMvQsE/GD50tIB0zDrAqNw+mGfmSc1MhrWkaPdTUDKEw8meYJK2azMymWrPrILIQujb6sNMWm5pr9+BlPAE8nW0amfI= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from imap1.dmz-prg2.suse.org (imap1.dmz-prg2.suse.org [IPv6:2a07:de40:b281:104: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 8F4A22257D for ; Mon, 26 Feb 2024 11:58:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1708948697; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=JIm2/oSNNpcwzx1/AzRn04uueKJPDEoD6jfGDVmcheU=; b=SR5wy6EYWSmVqCsjx18kVp6LbmH6NdEqoXehOqE1xCYI9indDMzaCQz/9xsgrlU5ro2SfT igVdkQ4jgeDWD8tISOfcCeCcoZIZl2ZPbdrPRVHxOC+gyZjyMwIyT1sNYnY/AQS2ek7D9h HgNAxFGNCZkhg50qgQCiApMdHox9hV8= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1708948697; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=JIm2/oSNNpcwzx1/AzRn04uueKJPDEoD6jfGDVmcheU=; b=UuWdlSSyuKlIMU5IQV+x9emz2oIP3sORFAlMtb/aN8qvXONz8gcYLoS3/9UpTYUhgk2bfq pGnFTr2q/KtBRiBw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1708948697; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=JIm2/oSNNpcwzx1/AzRn04uueKJPDEoD6jfGDVmcheU=; b=SR5wy6EYWSmVqCsjx18kVp6LbmH6NdEqoXehOqE1xCYI9indDMzaCQz/9xsgrlU5ro2SfT igVdkQ4jgeDWD8tISOfcCeCcoZIZl2ZPbdrPRVHxOC+gyZjyMwIyT1sNYnY/AQS2ek7D9h HgNAxFGNCZkhg50qgQCiApMdHox9hV8= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1708948697; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=JIm2/oSNNpcwzx1/AzRn04uueKJPDEoD6jfGDVmcheU=; b=UuWdlSSyuKlIMU5IQV+x9emz2oIP3sORFAlMtb/aN8qvXONz8gcYLoS3/9UpTYUhgk2bfq pGnFTr2q/KtBRiBw== 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 775E413A98 for ; Mon, 26 Feb 2024 11:58:17 +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 aAHLG9l83GU+RwAAD6G6ig (envelope-from ) for ; Mon, 26 Feb 2024 11:58:17 +0000 From: Tom de Vries To: gdb-patches@sourceware.org Subject: [PATCH v2 2/2] [gdb/dap] Fix stray KeyboardInterrupt after cancel Date: Mon, 26 Feb 2024 12:58:25 +0100 Message-Id: <20240226115825.29705-2-tdevries@suse.de> X-Mailer: git-send-email 2.35.3 In-Reply-To: <20240226115825.29705-1-tdevries@suse.de> References: <20240226115825.29705-1-tdevries@suse.de> MIME-Version: 1.0 X-Spam-Level: Authentication-Results: smtp-out1.suse.de; dkim=pass header.d=suse.de header.s=susede2_rsa header.b=SR5wy6EY; dkim=pass header.d=suse.de header.s=susede2_ed25519 header.b=UuWdlSSy X-Rspamd-Server: rspamd2.dmz-prg2.suse.org X-Spamd-Result: default: False [-1.51 / 50.00]; RCVD_VIA_SMTP_AUTH(0.00)[]; SPAMHAUS_XBL(0.00)[2a07:de40:b281:104:10:150:64:97:from]; R_MISSING_CHARSET(2.50)[]; TO_DN_NONE(0.00)[]; BROKEN_CONTENT_TYPE(1.50)[]; RCVD_COUNT_THREE(0.00)[3]; DKIM_TRACE(0.00)[suse.de:+]; MX_GOOD(-0.01)[]; NEURAL_HAM_SHORT(-0.20)[-1.000]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; BAYES_HAM(-3.00)[100.00%]; ARC_NA(0.00)[]; R_DKIM_ALLOW(-0.20)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; RCVD_DKIM_ARC_DNSWL_HI(-1.00)[]; FROM_HAS_DN(0.00)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000]; MIME_GOOD(-0.10)[text/plain]; PREVIOUSLY_DELIVERED(0.00)[gdb-patches@sourceware.org]; RCPT_COUNT_ONE(0.00)[1]; DKIM_SIGNED(0.00)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; MID_CONTAINS_FROM(1.00)[]; DBL_BLOCKED_OPENRESOLVER(0.00)[suse.de:dkim]; FUZZY_BLOCKED(0.00)[rspamd.com]; RCVD_IN_DNSWL_HI(-1.00)[2a07:de40:b281:104:10:150:64:97:from,2a07:de40:b281:106:10:150:64:167:received]; RCVD_TLS_ALL(0.00)[] X-Spam-Score: -1.51 X-Rspamd-Queue-Id: 8F4A22257D 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 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org 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, renaming variable in_flight to in_flight_dap_thread, and adding a variable in_flight_gdb_thread to be able to distinguish when a request is in flight in the dap thread or the gdb thread. - adding a wrapper Cancellable to to deal with cancelling the wrapped event - using Cancellable in send_gdb and send_gdb_with_response to wrap the posted event - in CancellationHandler.cancel, only call gdb.interrupt if req == self.in_flight_gdb_thread. 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. Tested on aarch64-linux. PR dap/31275 Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31275 --- gdb/python/lib/gdb/dap/server.py | 88 ++++++++++++++++++++++++-------- 1 file changed, 67 insertions(+), 21 deletions(-) diff --git a/gdb/python/lib/gdb/dap/server.py b/gdb/python/lib/gdb/dap/server.py index e957b886fb8..59f486f5729 100644 --- a/gdb/python/lib/gdb/dap/server.py +++ b/gdb/python/lib/gdb/dap/server.py @@ -19,6 +19,7 @@ import heapq import inspect import json import threading +from contextlib import contextmanager from .io import start_json_writer, read_json from .startup import ( @@ -59,24 +60,19 @@ class CancellationHandler: # Methods on this class acquire this lock before proceeding. self.lock = threading.Lock() # The request currently being handled, or None. - self.in_flight = None + self.in_flight_dap_thread = None + self.in_flight_gdb_thread = None self.reqs = [] def starting(self, req): - """Call at the start of the given request. - - Throws the appropriate exception if the request should be - immediately cancelled.""" + """Call at the start of the given request.""" with self.lock: - self.in_flight = req - while len(self.reqs) > 0 and self.reqs[0] <= req: - if heapq.heappop(self.reqs) == req: - raise KeyboardInterrupt() + self.in_flight_dap_thread = req def done(self, req): """Indicate that the request is done.""" with self.lock: - self.in_flight = None + self.in_flight_dap_thread = None def cancel(self, req): """Call to cancel a request. @@ -85,7 +81,7 @@ class CancellationHandler: If the request is in flight, it is interrupted. If the request has not yet been seen, the cancellation is queued.""" with self.lock: - if req == self.in_flight: + if req == self.in_flight_gdb_thread: gdb.interrupt() else: # We don't actually ignore the request here, but in @@ -96,6 +92,29 @@ class CancellationHandler: # to try to check for this. heapq.heappush(self.reqs, req) + @contextmanager + def interruptable_region(self, req): + """Return a new context manager that sets in_flight_gdb_thread to + REQ.""" + if req is None: + # No request is handled in the region, just execute the region. + yield None + return + with self.lock: + # If the request is cancelled, don't execute the region. + while len(self.reqs) > 0 and self.reqs[0] <= req: + if heapq.heappop(self.reqs) == req: + raise KeyboardInterrupt() + # Request is being handled by the gdb thread. + self.in_flight_gdb_thread = req + try: + # Execute region. This may be interrupted by gdb.interrupt. + yield None + finally: + # Request has been handled by the gdb thread, + with self.lock: + self.in_flight_gdb_thread = None + class Server: """The DAP server class.""" @@ -433,13 +452,45 @@ class Invoker(object): exec_and_log(self.cmd) +class Cancellable(object): + + def __init__(self, fn, result_q=None): + self.fn = fn + self.result_q = result_q + with _server.canceller.lock: + self.req = _server.canceller.in_flight_dap_thread + + # This is invoked in the gdb thread to run self.fn. + @in_gdb_thread + def __call__(self): + try: + with _server.canceller.interruptable_region(self.req): + val = self.fn() + if self.result_q is not None: + self.result_q.put(val) + except (Exception, KeyboardInterrupt) as e: + if self.result_q is not None: + # Pass result or exception to caller. + self.result_q.put(e) + elif isinstance(e, KeyboardInterrupt): + # Fn was cancelled. + pass + else: + # Exception happened. Ignore and log it. + err_string = "%s, %s" % (err, type(err)) + thread_log("caught exception: " + err_string) + log_stack() + + def send_gdb(cmd): """Send CMD to the gdb thread. CMD can be either a function or a string. If it is a string, it is passed to gdb.execute.""" if isinstance(cmd, str): cmd = Invoker(cmd) - gdb.post_event(cmd) + + # Post the event and don't wait for the result. + gdb.post_event(Cancellable(cmd)) def send_gdb_with_response(fn): @@ -451,17 +502,12 @@ def send_gdb_with_response(fn): """ if isinstance(fn, str): fn = Invoker(fn) - result_q = DAPQueue() - - def message(): - try: - val = fn() - result_q.put(val) - except (Exception, KeyboardInterrupt) as e: - result_q.put(e) - send_gdb(message) + # Post the event and wait for the result in result_q. + result_q = DAPQueue() + gdb.post_event(Cancellable(fn, result_q)) val = result_q.get() + if isinstance(val, (Exception, KeyboardInterrupt)): raise val return val