Re: [PATCH] Add a timeout parameter to gdb_do_one_event

[Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>]

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

?

> ---
>  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);
>  \f
>  /* 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

> +   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?

> +   The timeout is never used by gdb itself, it is however needed to
> +   integrate gdb event handling within some external (i.e.: GUI) event
> +   loop. */

Err, you can probably say "Insight" here.  It's not like we want to
support programs doing this in general.  We make an exception for
Insight because of historical reasons, I suppose.

>  int
> -gdb_do_one_event (void)
> +gdb_do_one_event (int mstimeout)
>  {
>    static int event_source_head = 0;
>    const int number_of_sources = 3;
>    int current = 0;
> +  int res = 0;
>  
>    /* First let's see if there are any asynchronous signal handlers
>       that are ready.  These would be the result of invoking any of the
> @@ -198,8 +203,6 @@ gdb_do_one_event (void)
>       round-robin fashion.  */
>    for (current = 0; current < number_of_sources; current++)
>      {
> -      int res;
> -
>        switch (event_source_head)
>  	{
>  	case 0:
> @@ -232,14 +235,30 @@ gdb_do_one_event (void)
>    /* Block waiting for a new event.  If gdb_wait_for_event returns -1,
>       we should get out because this means that there are no event
>       sources left.  This will make the event loop stop, and the
> -     application exit.  */
> -
> -  if (gdb_wait_for_event (1) < 0)
> -    return -1;
> +     application exit.
> +     If a timeout has been given, a new timer is set accordingly
> +     to abort event wait.  It is deleted upon gdb_wait_for_event
> +     termination and thus should never be triggered.
> +     When the timeout is reached, events are not monitored again:
> +     they already have been checked in the loop above. */
> +
> +  if (mstimeout != 0)
> +    {
> +      int timerid = 0;
> +
> +      if (mstimeout > 0)
> +	timerid = create_timer (mstimeout,
> +				[] (gdb_client_data timeridp)
> +				  {
> +				    *((int *) timeridp) = 0;
> +				  },
> +				&timerid);
> +      res = gdb_wait_for_event (1);
> +      if (timerid)
> +	delete_timer (timerid);

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.

Simon