From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id YPP7CgywJ2EAMAAAWB0awg (envelope-from ) for ; Thu, 26 Aug 2021 11:15:24 -0400 Received: by simark.ca (Postfix, from userid 112) id 29AF31EE1B; Thu, 26 Aug 2021 11:15:24 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: * X-Spam-Status: No, score=1.4 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,URIBL_BLOCKED,URIBL_DBL_SPAM autolearn=no autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 874621EDEE for ; Thu, 26 Aug 2021 11:15:23 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 0F2473857C60 for ; Thu, 26 Aug 2021 15:15:23 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 0F2473857C60 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1629990923; bh=cxp9wcciL1+gTBL8FU0o/t49RCiJNFF1oZ0X1xepd3k=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=bX7fLOrD5D7H4oKmS1s25rN1LGB/CHTbxbouALqWGELJ5vKSvfLqzDCeAEz3WDTym IDCOTeeSjRsxMJZynUv8BHIsaDViQdn/v0uauDh6J8yAmtqFDZjXqEeAk1b86R/7l9 XHZFg7VApAtvpoa5mH5DLL5fEUybhk7sqsY9zhHU= Received: from jupiter.monnerat.net (jupiter.monnerat.net [46.226.111.226]) by sourceware.org (Postfix) with ESMTPS id 5BDFF3858D3C for ; Thu, 26 Aug 2021 15:14:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 5BDFF3858D3C Received: from patrick.monnerat ([192.168.0.128]) by jupiter.monnerat.net (8.14.8/8.14.8) with ESMTP id 17QFEaJn010739 (version=TLSv1/SSLv3 cipher=AES256-GCM-SHA384 bits=256 verify=OK); Thu, 26 Aug 2021 17:14:41 +0200 DKIM-Filter: OpenDKIM Filter v2.10.3 jupiter.monnerat.net 17QFEaJn010739 Subject: Re: [PATCH] Add a timeout parameter to gdb_do_one_event To: Simon Marchi , gdb-patches@sourceware.org References: <20210823182359.104456-1-patrick@monnerat.net> <4e3085bb-af40-e0dc-73aa-991f97243e06@polymtl.ca> <60a52fef-89f0-62f1-1a45-5e5a40f47df6@monnerat.net> <12e53aee-05ae-8060-df57-0fc722d4a82c@polymtl.ca> Message-ID: <0442ef6e-bb22-924d-a1ef-05d6e29cf5f0@monnerat.net> Date: Thu, 26 Aug 2021 17:14:36 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <12e53aee-05ae-8060-df57-0fc722d4a82c@polymtl.ca> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US 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: , From: Patrick Monnerat via Gdb-patches Reply-To: Patrick Monnerat Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" On 8/26/21 3:47 PM, Simon Marchi wrote: > > On 2021-08-26 7:36 a.m., Patrick Monnerat wrote: >> On 8/26/21 5:24 AM, Simon Marchi wrote: >>> On 2021-08-23 2:23 p.m., Patrick Monnerat via Gdb-patches wrote: >>>> Since commit b2d8657, having a per-interpreter event/command loop is not >>>> possible anymore. >>>> >>>> As Insight uses a GUI that has its own event loop, gdb and GUI event >>>> loops have then to be "merged" (i.e.: work together). But this is >>>> problematic as gdb_do_one_event is not aware of this alternate event >>>> loop and thus may wait forever. >>>> >>>> The solution is to implement a wait timeout to gdb_do_one_event. This >>>> cannot be done externally as timers are event sources themselves. >>>> >>>> The new parameter defaults to "no timeout": as it is used by Insight >>>> only, there is no need to update calls from the gdb source tree. >>> So, Insight's main loop looks like: >>> >>> while True: >>> call gdb_do_one_event with a timeout >>> call gui_do_one_event with a timeout >>> >>> ? >> Not exactly, although this is the first idea that emerges. But this approach is not reactive enough and consumes CPU uselessly. > Indeed, happy to know it's not that. > >> The real implementation makes the GUI event loop call gdb_do_one_event and recursively. The actual event waiting is performed by gdb_do_one_event, but the GUI may define a timeout for it. The hard task here is to avoid infinite recursion. >> >> As Insight GUI is Tcl/Tk, a Tcl C API feature called a notifier (https://www.tcl.tk/man/tcl8.4/TclLib/Notifier.html) allowed me to design such a strategy. See https://sourceware.org/git/?p=insight.git;a=blob;f=gdbtk/generic/gdbtk.c line 247 and under to satisfy your curiosity! But it is a quite large part of the interface between the 2 subsystems that was not needed before gdb commit b2d8657. Note that an additional patch (unsubmitted yet) is needed to map Tcl file events into gdb file handlers. > Ok, so you use GDB's event loop, registering some additional events in > it. For example, the user clicking a button will generate an event, > wake up gdb_wait_for_event and call your handler to go do the GUI work. > Is that it? To be short, yes. But a mouse click is a bad example as it is not handled exactly the same way in Unixes and W$. Tcl "*events" are also timed calls (with their own queue) and "idle" (something that should be executed whenever no event is pending). The Tcl notifier is informed by the interpreter of the max time to wait, taking into account these 2 event sources. I tried to implement this timeout with a gdb timer created before calling gdb_wait_for_event, but this failed as, if 0, it may be triggered before any other pending event because of the round-robin strategy of gdb_do_one_event initial loop. > > And the timeout is needed to call into the GUI subsystem to do some > periodic work? That's it. And as noted above, the actual timeout value is determined by Tcl. > >>>> --- >>>> gdbsupport/event-loop.cc | 45 ++++++++++++++++++++++++++++------------ >>>> gdbsupport/event-loop.h | 2 +- >>>> 2 files changed, 33 insertions(+), 14 deletions(-) >>>> >>>> diff --git a/gdbsupport/event-loop.cc b/gdbsupport/event-loop.cc >>>> index 98d1ada52cd..72c64dcdb72 100644 >>>> --- a/gdbsupport/event-loop.cc >>>> +++ b/gdbsupport/event-loop.cc >>>> @@ -177,16 +177,21 @@ static int update_wait_timeout (void); >>>> static int poll_timers (void); >>>> >>>> /* Process one high level event. If nothing is ready at this time, >>>> - wait for something to happen (via gdb_wait_for_event), then process >>>> - it. Returns >0 if something was done otherwise returns <0 (this >>>> - can happen if there are no event sources to wait for). */ >>>> + wait at most mstimeout milliseconds for something to happen (via >>> mstimeout -> MSTIMEOUT >> Even if not a constant but the name of a parameter? > Yes, if you look around, this is how we refer to parameter names in > comment. Like it or hate it, that's how it is. No problem: I just wanted to be sure it was not a misunderstanding. > >>>> + gdb_wait_for_event), then process it. Returns >0 if something was >>>> + done, <0 if there are no event sources to wait for, =0 if timeout occurred. >>>> + Setting the timeout to a negative value disables it. >>> Does setting the timeout to 0 mean return immediately if nothing is >>> available? >> Yes, > Ok, can you mention it in the comment? I don't think it is mentioned. Will do! > >>> Probably not a practical concern, but by creating timers over and over, >>> for a reaaaaaaally long GDB session, the timer id will eventually wrap >>> and create_timer might return 0. I don't know what will happen then. >> Yes, you're right. I'll change it. Maybe an RAII class here too? > Not sure, event with an RAII class you need a way to flag whether a > timer was created or not. Maybe optional? > > gdb::optional timer_id; > > if (mstimeout > 0) > timer_id = create_timer (...); > > res = gdb_wait_for_event (1); > > if (timer_id.has_value ()) > delete_timer (*timer_id); > Thanks for the hint. I think both together would be optimal. Patrick