From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id GewdC9h3KWWL3CwAWB0awg (envelope-from ) for ; Fri, 13 Oct 2023 13:01:12 -0400 Authentication-Results: simark.ca; dkim=pass (2048-bit key; secure) header.d=lancelotsix.com header.i=@lancelotsix.com header.a=rsa-sha256 header.s=2021 header.b=Pp/oNJto; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 239F51E0C1; Fri, 13 Oct 2023 13:01:12 -0400 (EDT) Received: from server2.sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 12B021E091 for ; Fri, 13 Oct 2023 13:01:10 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id AC81138582A4 for ; Fri, 13 Oct 2023 17:01:09 +0000 (GMT) Received: from lndn.lancelotsix.com (lndn.lancelotsix.com [51.195.220.111]) by sourceware.org (Postfix) with ESMTPS id 739A03858D1E for ; Fri, 13 Oct 2023 17:00:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 739A03858D1E Authentication-Results: sourceware.org; dmarc=pass (p=reject dis=none) header.from=lancelotsix.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=lancelotsix.com Received: from octopus (cust120-dsl54.idnet.net [212.69.54.120]) by lndn.lancelotsix.com (Postfix) with ESMTPSA id 7E5CC864F6; Fri, 13 Oct 2023 17:00:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=lancelotsix.com; s=2021; t=1697216454; bh=+RszlzRqUB39w4BKASurB1B7OEh3FZ5pK88Im40Y/o0=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Pp/oNJtoULDMK0kG9MwcR2xLOmHXpUbDlOZwFPpleO74gRQ4ZJWD9swFpZiLZtkPL KTcbBiRRZqkuoI/Wjldky4X1jqrCNqZzdPB2zDWxUjRCbq9wo43SWeZGxLGiQQ1qDc kzfrh3Fe6DxgGWwurUx0IdFwFfzxCwA2JW8/Z70AI5cGrGYC5Ke0SGl/oZUAJSA2Hw kIqjRXB+0vMI0pyBRsykesNYiVUizi/uImI6cTxu1FeqADWCOaEs08PSa7qlQkU7jK Sn8m2KRdgY25H1qN2tKn2BGSSINaHza/8mBLQa5zZbEB6ERLqQkGzI4Td6zwseZenu n9N1drJXbdh0A== Date: Fri, 13 Oct 2023 18:00:50 +0100 From: Lancelot SIX To: Tom de Vries Cc: gdb-patches@sourceware.org Subject: Re: [PATCH v2 4/4] [gdb/cli] Allow source highlighting to be interrupted Message-ID: <20231013170026.245lqdb5vg4m4lcp@octopus> References: <20231013121953.25917-1-tdevries@suse.de> <20231013121953.25917-5-tdevries@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20231013121953.25917-5-tdevries@suse.de> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.6.2 (lndn.lancelotsix.com [0.0.0.0]); Fri, 13 Oct 2023 17:00:54 +0000 (UTC) X-Spam-Status: No, score=-11.9 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 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+public-inbox=simark.ca@sourceware.org Hi Tom, I have a comment below. On Fri, Oct 13, 2023 at 02:19:53PM +0200, Tom de Vries wrote: > In PR cli/30934, a problem is reported where gdb becomes unresponsive for > 2m13s because the GNU source-highlight library is taking a lot of time to > annotate the source. > > This is due to a problem in the library [1], for which I've posted a > patch [2], which brings down the annotation time to 3s. > > However, GDB should not become unresponsive due to such a problem. > > Fix this by installing a srchilite::HighlightEventListener that: > - checks whether ^C was pressed, and if so > - asks the user whether it should interrupt highlighting, and if so > - abandons the highlighting by throwing an exception. > > Interrupting the highlighting looks like this to the user: > ... > $ gdb -q a.out -ex "b 56" > Reading symbols from a.out... > Breakpoint 1 at 0x400e2a: file test.cpp, line 56. > (gdb) r > Starting program: /data/vries/gdb/a.out > > Breakpoint 1, Solution::numOfSubarrays () at test.cpp:56 > ^CCancel source styling using GNU source highlight of /data/vries/gdb/test.cpp? > ([y] or n) y > 56 return (int) t; > (gdb) > ... > Note that line 56 still can be highlighted, just by pygments instead of > source-highlight. > > Tested on x86_64-linux. > > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30934 > > [1] https://savannah.gnu.org/bugs/?64747 > [2] https://savannah.gnu.org/patch/index.php?10400 > --- > gdb/source-cache.c | 62 ++++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 60 insertions(+), 2 deletions(-) > > diff --git a/gdb/source-cache.c b/gdb/source-cache.c > index f8c98d7e23f..57d9f3b8ab4 100644 > --- a/gdb/source-cache.c > +++ b/gdb/source-cache.c > @@ -223,9 +266,21 @@ try_source_highlight (std::string &contents ATTRIBUTE_UNUSED, > highlighter->setStyleFile ("esc.style"); > } > > + gdb_highlight_event_listener event_listener (fullname); > + highlighter->setHighlightEventListener (&event_listener); In this instance, highlighter outlives event_listener but keeps a reference to it. Arguably, the event listener will be reset to point to another instance before the next invocation of highlight, but I think it would be cleaner to make sure that the event listener is cleared anyway: auto clear_event_listener = make_scope_exit ([highlighter]() { highlighter->setHighlightEventListener (nullptr); }); Who knows, maybe one day highlighter's dtor might want to notify something. WDYT? Best, Lancelot. > + > std::istringstream input (contents); > std::ostringstream output; > - highlighter->highlight (input, output, lang_name, fullname); > + { > + /* We want to be able to interrupt the call to source-highlight. If > + the current quit_handler is infrun_quit_handler, pressing ^C is > + ignored by QUIT (assuming target_terminal::is_ours), so install the > + default quit handler. */ > + scoped_restore restore_quit_handler > + = make_scoped_restore (&quit_handler, default_quit_handler); > + > + highlighter->highlight (input, output, lang_name, fullname); > + } > #if __cplusplus >= 202002L > contents = std::move (output).str(); > #else > @@ -308,13 +363,16 @@ source_cache::ensure (struct symtab *s) > reasons: > - the language is not supported. > - the language cannot not be auto-detected from the file name. > + - styling took too long and was interrupted by the user. > - no stylers available. > > Since styling failed, don't try styling the file again after it > drops from the cache. > > Note that clearing the source cache also clears > - m_no_styling_files. */ > + m_no_styling_files, so if styling took too long, and the user > + interrupted it, and the source cache gets cleared, the user will > + need to interrupt styling again. */ > m_no_styling_files.insert (fullname); > } > } > -- > 2.35.3 >