generic async event handlers in the event loop, for remote non-stop (was: generic `struct serial' interface pipe for remote non-stop)

generic async event handlers in the event loop, for remote non-stop (was: generic `struct serial' interface pipe for remote non-stop)

[Pedro Alves]

[-- Attachment #1: Type: text/plain, Size: 2840 bytes --]

Hi guys,

I'm going to apply the attached patch, which is a cleanup of the patch I posted
here:

 http://sourceware.org/ml/gdb-patches/2008-10/msg00449.html

This version, instead of overloading/abusing async_signal_handlers, comes up
with a similar (although not quite the same) async_event_handlers concept.

These async events are really very similar in implementation to
async_signal_handlers, except that, signal handlers should keep the current
behaviour of having high priority in relation to normal events (I changed
that before), while async_event_handlers should compete in the same weight
as the monitored file descriptors in the round-robin event source selection.

Then, I'm going to use async_event_handlers instead of async_signal_handlers
in gdb/remote.c for the extra target event sources in remote non-stop.
(that's just s/async_signal/async_event/g on top of the remote.c hunk I
had posted in the link above, I'll post an updated patch shortly.)

-- 
Pedro Alves

On Friday 17 October 2008 21:54:49, Pedro Alves wrote:

> That being said, I took a step back, and relooked at the problems I was
> having with async signals handlers, which is a mechanism to register a 
> callback function in the event-loop.  Notice that the currently event
> loop design is heavilly based on waitable file descriptors, with async
> signal handlers being a secondary input event source.
> 
> So I came up with the attached patch to adjust it to my needs.
> 
> The biggest problem the attached patch solves is:
> 
>  - Since async signal handlers are always checked before polling the
>    file descriptors, I can easily starve the file descriptor based
>    sources.  E.g., if I place several threads displace stepping a
>    breakpoint, forcing continuous remote traffic, the CLI becomes very,
>    very unresponsive, unusable really.  This is because stdin itself
>    is registered as a monitored (via select/poll) file descriptor.
> 
>  To solve this, I adjusted the event loop to give equal priority to
>  all event-sources (timers, async signal handlers, monitored file
>  descriptors).  The async signal handling was changed to instead
>  of immediatelly calling the associated callback, it installs an
>  event in the "ready" queue, just like the file-descriptor and
>  timer based sources.  I then make sure that I poll each of the
>  possible event sources once (select/poll with timeout 0 too).  If
>  if no event is found ready, then, we go blocking waiting for one
>  in select/poll.
> 
> I can sucessfully debug a non-stop linux gdbserver from both
> a linux host and a Windows (mingw32) host with this.
> 
> The remote.c changes below apply on top of the non-stop support
> patch I sent yesterday, and it's what I used to test, shown here
> so you could see what was required that change, in case you're
> curious about it.

[-- Attachment #2: event_loop.diff --]
[-- Type: text/x-diff, Size: 24682 bytes --]

l2008-10-24  Pedro Alves  <pedro@codesourcery.com>

	* event-loop.h: Mention async_event_handlers.
	(async_event_handler): Forward declare.
	(async_event_handler_func): New typedef.
	(create_async_event_handler, delete_async_event_handler)
	(mark_async_event_handler): Declare.

	* event-loop.c (event_data): New.
	(event_handler_func): Take an event_data instead of an integer.
	(struct gdb_event): Replace the integer file descriptor by a
	generic event_data.
	(async_event_handler): New.
	(async_handler_ready): Delete.
	(async_event_handler_list): New.
	(create_event): New.
	(create_file_event): Use it.
	(process_event): Adjust.
	(gdb_do_one_event): Poll from the event sources in round-robin
	fashion across calls.  Be sure to consult all sources before
	blocking.
	(handle_file_event): Take an event_data instead of an integer.
	Adjust.
	(gdb_wait_for_event): Add `block' argument.  Handle it.
	(mark_async_signal_handler): Remove unneeded cast.
	(invoke_async_signal_handler): Rename to ...
	(invoke_async_signal_handlres): ... this.  Return true if any was
	handled.
	(check_async_ready): Delete
	(create_async_event_handler): New.
	(mark_async_event_handler): New.
	(struct async_event_handler_data): New.
	(invoke_async_event_handler): New.
	(check_async_event_handlers): New.
	(delete_async_event_handler): New.
	(handle_timer_event): Adjust.

---
 gdb/event-loop.c |  383 +++++++++++++++++++++++++++++++++++++++++--------------
 gdb/event-loop.h |   52 +++++--
 2 files changed, 330 insertions(+), 105 deletions(-)

Index: src/gdb/event-loop.h
===================================================================
--- src.orig/gdb/event-loop.h	2008-10-24 01:19:55.000000000 +0100
+++ src/gdb/event-loop.h	2008-10-24 01:19:57.000000000 +0100
@@ -24,21 +24,33 @@
    sources to listen on.  External event sources can be plugged into
    the loop.
 
-   There are 3 main components: 
+   There are 4 main components:
    - a list of file descriptors to be monitored, GDB_NOTIFIER.  
+   - a list of asynchronous event sources to be monitored,
+     ASYNC_EVENT_HANDLER_LIST.
    - a list of events that have occurred, EVENT_QUEUE.  
    - a list of signal handling functions, SIGHANDLER_LIST.
 
-   GDB_NOTIFIER keeps track of the event sources. Event sources for
-   gdb are currently the UI and the target.  Gdb communicates with the
-   command line user interface via the readline library and usually
-   communicates with remote targets via a serial port. Serial ports
-   are represented in GDB as file descriptors and select/poll calls.
-   For native targets instead, the communication consists of calls to
-   ptrace and waits (via signals) or calls to poll/select (via file
-   descriptors). In the current gdb, the code handling events related
-   to the target resides in the wait_for_inferior function and in
-   various target specific files (*-tdep.c).
+   GDB_NOTIFIER keeps track of the file descriptor based event
+   sources.  ASYNC_EVENT_HANDLER_LIST keeps track of asynchronous
+   event sources that are signalled by some component of gdb, usually
+   a target_ops instance.  Event sources for gdb are currently the UI
+   and the target.  Gdb communicates with the command line user
+   interface via the readline library and usually communicates with
+   remote targets via a serial port.  Serial ports are represented in
+   GDB as file descriptors and select/poll calls.  For native targets
+   instead, the communication varies across operating system debug
+   APIs, but usually consists of calls to ptrace and waits (via
+   signals) or calls to poll/select (via file descriptors).  In the
+   current gdb, the code handling events related to the target resides
+   in wait_for_inferior for synchronous targets; or, for asynchronous
+   capable targets, by having the target register either a target
+   controlled file descriptor and/or an asynchronous event source in
+   the event loop, with the fetch_inferior_event function as the event
+   callback.  In both the synchronous and asynchronous cases, usually
+   the target event is collected through the target_wait interface.
+   The target is free to install other event sources in the event loop
+   if it so requires.
 
    EVENT_QUEUE keeps track of the events that have happened during the
    last iteration of the event loop, and need to be processed.  An
@@ -57,8 +69,10 @@
 
 typedef void *gdb_client_data;
 struct async_signal_handler;
+struct async_event_handler;
 typedef void (handler_func) (int, gdb_client_data);
 typedef void (sig_handler_func) (gdb_client_data);
+typedef void (async_event_handler_func) (gdb_client_data);
 typedef void (timer_handler_func) (gdb_client_data);
 
 /* Where to add an event onto the event queue, by queue_event. */
@@ -113,3 +127,19 @@ void mark_async_signal_handler (struct a
 
 void gdb_call_async_signal_handler (struct async_signal_handler *handler,
 				    int immediate_p);
+
+/* Create and register an asynchronous event source in the event loop,
+   and set PROC as its callback.  CLIENT_DATA is passed as argument to
+   PROC upon its invocation.  Returns a pointer to an opaque structure
+   used to mark as ready and to later delete this event source from
+   the event loop.  */
+extern struct async_event_handler *
+  create_async_event_handler (async_event_handler_func *proc, gdb_client_data client_data);
+
+/* Remove the event source pointed by HANDLER_PTR created by
+   CREATE_ASYNC_EVENT_HANDLER from the event loop, and release it.  */
+extern void delete_async_event_handler (struct async_event_handler **handler_ptr);
+
+/* Call the handler from HANDLER the next time through the event
+   loop.  */
+extern void mark_async_event_handler (struct async_event_handler *handler);
Index: src/gdb/event-loop.c
===================================================================
--- src.orig/gdb/event-loop.c	2008-10-24 01:19:55.000000000 +0100
+++ src/gdb/event-loop.c	2008-10-24 01:19:57.000000000 +0100
@@ -38,25 +38,40 @@
 #include "gdb_assert.h"
 #include "gdb_select.h"
 
+/* Data point to pass to the event handler.  */
+typedef union event_data
+{
+  void *ptr;
+  int integer;
+} event_data;
+
 typedef struct gdb_event gdb_event;
-typedef void (event_handler_func) (int);
+typedef void (event_handler_func) (event_data);
 
 /* Event for the GDB event system.  Events are queued by calling
    async_queue_event and serviced later on by gdb_do_one_event. An
    event can be, for instance, a file descriptor becoming ready to be
-   read. Servicing an event simply means that the procedure PROC will
+   read.  Servicing an event simply means that the procedure PROC will
    be called.  We have 2 queues, one for file handlers that we listen
    to in the event loop, and one for the file handlers+events that are
-   ready. The procedure PROC associated with each event is always the
-   same (handle_file_event).  Its duty is to invoke the handler
-   associated with the file descriptor whose state change generated
-   the event, plus doing other cleanups and such. */
+   ready. The procedure PROC associated with each event is dependant
+   of the event source.  In the case of monitored file descriptors, it
+   is always the same (handle_file_event).  Its duty is to invoke the
+   handler associated with the file descriptor whose state change
+   generated the event, plus doing other cleanups and such.  In the
+   case of async signal handlers, it is
+   invoke_async_signal_handler.  */
 
 struct gdb_event
   {
-    event_handler_func *proc;	/* Procedure to call to service this event. */
-    int fd;			/* File descriptor that is ready. */
-    struct gdb_event *next_event;	/* Next in list of events or NULL. */
+    /* Procedure to call to service this event.  */
+    event_handler_func *proc;
+
+    /* Data to pass to the event handler.  */
+    event_data data;
+
+    /* Next in list of events or NULL.  */
+    struct gdb_event *next_event;
   };
 
 /* Information about each file descriptor we register with the event
@@ -82,7 +97,7 @@ file_handler;
    be carried out by PROC at a later time, within process_event. This
    provides a deferred execution of signal handlers.
    Async_init_signals takes care of setting up such an
-   asyn_signal_handler for each interesting signal. */
+   async_signal_handler for each interesting signal. */
 typedef struct async_signal_handler
   {
     int ready;			/* If ready, call this handler from the main event loop, 
@@ -93,6 +108,29 @@ typedef struct async_signal_handler
   }
 async_signal_handler;
 
+/* PROC is a function to be invoked when the READY flag is set.  This
+   happens when the event has been marked with
+   MARK_ASYNC_EVENT_HANDLER.  The actual work to be done in response
+   to an event will be carried out by PROC at a later time, within
+   process_event.  This provides a deferred execution of event
+   handlers.  */
+typedef struct async_event_handler
+  {
+    /* If ready, call this handler from the main event loop, using
+       invoke_event_handler.  */
+    int ready;
+
+    /* Point to next handler.  */
+    struct async_event_handler *next_handler;
+
+    /* Function to call to do the work.  */
+    async_event_handler_func *proc;
+
+    /* Argument to PROC.  */
+    gdb_client_data client_data;
+  }
+async_event_handler;
+
 
 /* Event queue:  
    - the first event in the queue is the head of the queue. 
@@ -207,21 +245,25 @@ static struct
   }
 sighandler_list;
 
-/* Are any of the handlers ready?  Check this variable using
-   check_async_ready. This is used by process_event, to determine
-   whether or not to invoke the invoke_async_signal_handler
-   function. */
-static int async_handler_ready = 0;
-
-static void create_file_handler (int fd, int mask, handler_func * proc, gdb_client_data client_data);
-static void invoke_async_signal_handler (void);
-static void handle_file_event (int event_file_desc);
-static int gdb_wait_for_event (void);
-static int check_async_ready (void);
-static void async_queue_event (gdb_event * event_ptr, queue_position position);
-static gdb_event *create_file_event (int fd);
-static int process_event (void);
-static void handle_timer_event (int dummy);
+/* All the async_event_handlers gdb is interested in are kept onto
+   this list. */
+static struct
+  {
+    /* Pointer to first in handler list. */
+    async_event_handler *first_handler;
+
+    /* Pointer to last in handler list. */
+    async_event_handler *last_handler;
+  }
+async_event_handler_list;
+
+static int invoke_async_signal_handlers (void);
+static void create_file_handler (int fd, int mask, handler_func * proc,
+				 gdb_client_data client_data);
+static void handle_file_event (event_data data);
+static void check_async_event_handlers (void);
+static void check_async_signal_handlers (void);
+static int gdb_wait_for_event (int);
 static void poll_timers (void);
 \f
 
@@ -260,6 +302,22 @@ async_queue_event (gdb_event * event_ptr
     }
 }
 
+/* Create a generic event, to be enqueued in the event queue for
+   processing.  PROC is the procedure associated to the event.  DATA
+   is passed to PROC uppon PROC invocation.  */
+
+static gdb_event *
+create_event (event_handler_func proc, event_data data)
+{
+  gdb_event *event;
+
+  event = xmalloc (sizeof (*event));
+  event->proc = proc;
+  event->data = data;
+
+  return event;
+}
+
 /* Create a file event, to be enqueued in the event queue for
    processing. The procedure associated to this event is always
    handle_file_event, which will in turn invoke the one that was
@@ -267,12 +325,10 @@ async_queue_event (gdb_event * event_ptr
 static gdb_event *
 create_file_event (int fd)
 {
-  gdb_event *file_event_ptr;
+  event_data data;
 
-  file_event_ptr = (gdb_event *) xmalloc (sizeof (gdb_event));
-  file_event_ptr->proc = handle_file_event;
-  file_event_ptr->fd = fd;
-  return (file_event_ptr);
+  data.integer = fd;
+  return create_event (handle_file_event, data);
 }
 
 /* Process one event.
@@ -280,7 +336,7 @@ create_file_event (int fd)
    or an asynchronous event handler can be invoked in response to
    the reception of a signal.
    If an event was processed (either way), 1 is returned otherwise
-   0 is returned.   
+   0 is returned.
    Scan the queue from head to tail, processing therefore the high
    priority events first, by invoking the associated event handler
    procedure. */
@@ -289,17 +345,14 @@ process_event (void)
 {
   gdb_event *event_ptr, *prev_ptr;
   event_handler_func *proc;
-  int fd;
+  event_data data;
 
   /* First let's see if there are any asynchronous event handlers that
      are ready. These would be the result of invoking any of the
      signal handlers. */
 
-  if (check_async_ready ())
-    {
-      invoke_async_signal_handler ();
-      return 1;
-    }
+  if (invoke_async_signal_handlers ())
+    return 1;
 
   /* Look in the event queue to find an event that is ready
      to be processed. */
@@ -310,7 +363,7 @@ process_event (void)
       /* Call the handler for the event. */
 
       proc = event_ptr->proc;
-      fd = event_ptr->fd;
+      data = event_ptr->data;
 
       /* Let's get rid of the event from the event queue.  We need to
          do this now because while processing the event, the proc
@@ -338,7 +391,7 @@ process_event (void)
       xfree (event_ptr);
 
       /* Now call the procedure associated with the event. */
-      (*proc) (fd);
+      (*proc) (data);
       return 1;
     }
 
@@ -355,33 +408,59 @@ process_event (void)
 int
 gdb_do_one_event (void *data)
 {
-  /* Any events already waiting in the queue? */
+  static int event_source_head = 0;
+  static const int number_of_sources = 3;
+  int current = 0;
+
+  /* Any events already waiting in the queue?  */
   if (process_event ())
+    return 1;
+
+  /* To level the fairness across event sources, we poll them in a
+     round-robin fashion.  */
+  for (current = 0; current < number_of_sources; current++)
     {
-      return 1;
+      switch (event_source_head)
+	{
+	case 0:
+	  /* Are any timers that are ready? If so, put an event on the
+	     queue. */
+	  poll_timers ();
+	  break;
+	case 1:
+	  /* Are there events already waiting to be collected on the
+	     monitored file descriptors?  */
+	  gdb_wait_for_event (0);
+	  break;
+	case 2:
+	  /* Are there any asynchronous event handlers ready?  */
+	  check_async_event_handlers ();
+	  break;
+	}
+
+      event_source_head++;
+      if (event_source_head == number_of_sources)
+	event_source_head = 0;
     }
 
-  /* Are any timers that are ready? If so, put an event on the queue. */
-  poll_timers ();
+  /* Handle any new events collected.  */
+  if (process_event ())
+    return 1;
 
-  /* Wait 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. */
+  /* 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 () < 0)
-    {
-      return -1;
-    }
+  if (gdb_wait_for_event (1) < 0)
+    return -1;
 
-  /* Handle any new events occurred while waiting. */
+  /* Handle any new events occurred while waiting.  */
   if (process_event ())
-    {
-      return 1;
-    }
+    return 1;
 
-  /* If gdb_wait_for_event has returned 1, it means that one
-     event has been handled. We break out of the loop. */
+  /* If gdb_wait_for_event has returned 1, it means that one event has
+     been handled.  We break out of the loop.  */
   return 1;
 }
 
@@ -659,7 +738,7 @@ delete_file_handler (int fd)
    through event_ptr->proc.  EVENT_FILE_DESC is file descriptor of the
    event in the front of the event queue. */
 static void
-handle_file_event (int event_file_desc)
+handle_file_event (event_data data)
 {
   file_handler *file_ptr;
   int mask;
@@ -667,6 +746,7 @@ handle_file_event (int event_file_desc)
   int error_mask;
   int error_mask_returned;
 #endif
+  int event_file_desc = data.integer;
 
   /* Search the file handler list to find one that matches the fd in
      the event. */
@@ -735,15 +815,13 @@ handle_file_event (int event_file_desc)
     }
 }
 
-/* Called by gdb_do_one_event to wait for new events on the 
-   monitored file descriptors. Queue file events as they are 
-   detected by the poll. 
-   If there are no events, this function will block in the 
-   call to poll.
-   Return -1 if there are no files descriptors to monitor, 
-   otherwise return 0. */
+/* Called by gdb_do_one_event to wait for new events on the monitored
+   file descriptors.  Queue file events as they are detected by the
+   poll.  If BLOCK and if there are no events, this function will
+   block in the call to poll.  Return -1 if there are no files
+   descriptors to monitor, otherwise return 0. */
 static int
-gdb_wait_for_event (void)
+gdb_wait_for_event (int block)
 {
   file_handler *file_ptr;
   gdb_event *file_event_ptr;
@@ -760,13 +838,18 @@ gdb_wait_for_event (void)
   if (use_poll)
     {
 #ifdef HAVE_POLL
-      num_found =
-	poll (gdb_notifier.poll_fds,
-	      (unsigned long) gdb_notifier.num_fds,
-	      gdb_notifier.timeout_valid ? gdb_notifier.poll_timeout : -1);
+      int timeout;
+
+      if (block)
+	timeout = gdb_notifier.timeout_valid ? gdb_notifier.poll_timeout : -1;
+      else
+	timeout = 0;
+
+      num_found = poll (gdb_notifier.poll_fds,
+			(unsigned long) gdb_notifier.num_fds, timeout);
 
       /* Don't print anything if we get out of poll because of a
-         signal. */
+	 signal.  */
       if (num_found == -1 && errno != EINTR)
 	perror_with_name (("poll"));
 #else
@@ -776,6 +859,18 @@ gdb_wait_for_event (void)
     }
   else
     {
+      struct timeval select_timeout;
+
+      struct timeval *timeout_p;
+      if (block)
+	timeout_p = gdb_notifier.timeout_valid
+	  ? &gdb_notifier.select_timeout : NULL;
+      else
+	{
+	  memset (&select_timeout, 0, sizeof (select_timeout));
+	  timeout_p = &select_timeout;
+	}
+
       gdb_notifier.ready_masks[0] = gdb_notifier.check_masks[0];
       gdb_notifier.ready_masks[1] = gdb_notifier.check_masks[1];
       gdb_notifier.ready_masks[2] = gdb_notifier.check_masks[2];
@@ -783,8 +878,7 @@ gdb_wait_for_event (void)
 			      &gdb_notifier.ready_masks[0],
 			      &gdb_notifier.ready_masks[1],
 			      &gdb_notifier.ready_masks[2],
-			      gdb_notifier.timeout_valid
-			      ? &gdb_notifier.select_timeout : NULL);
+			      timeout_p);
 
       /* Clear the masks after an error from select. */
       if (num_found == -1)
@@ -792,7 +886,9 @@ gdb_wait_for_event (void)
 	  FD_ZERO (&gdb_notifier.ready_masks[0]);
 	  FD_ZERO (&gdb_notifier.ready_masks[1]);
 	  FD_ZERO (&gdb_notifier.ready_masks[2]);
-	  /* Dont print anything is we got a signal, let gdb handle it. */
+
+	  /* Dont print anything if we got a signal, let gdb handle
+	     it.  */
 	  if (errno != EINTR)
 	    perror_with_name (("select"));
 	}
@@ -911,21 +1007,18 @@ call_async_signal_handler (struct async_
 void
 mark_async_signal_handler (async_signal_handler * async_handler_ptr)
 {
-  ((async_signal_handler *) async_handler_ptr)->ready = 1;
-  async_handler_ready = 1;
+  async_handler_ptr->ready = 1;
 }
 
-/* Call all the handlers that are ready. */
-static void
-invoke_async_signal_handler (void)
+/* Call all the handlers that are ready.  Returns true if any was
+   indeed ready.  */
+static int
+invoke_async_signal_handlers (void)
 {
   async_signal_handler *async_handler_ptr;
+  int any_ready = 0;
 
-  if (async_handler_ready == 0)
-    return;
-  async_handler_ready = 0;
-
-  /* Invoke ready handlers. */
+  /* Invoke ready handlers.  */
 
   while (1)
     {
@@ -938,13 +1031,15 @@ invoke_async_signal_handler (void)
 	}
       if (async_handler_ptr == NULL)
 	break;
+      any_ready = 1;
       async_handler_ptr->ready = 0;
       (*async_handler_ptr->proc) (async_handler_ptr->client_data);
     }
 
-  return;
+  return any_ready;
 }
 
+
 /* Delete an asynchronous handler (ASYNC_HANDLER_PTR). 
    Free the space allocated for it.  */
 void
@@ -971,11 +1066,111 @@ delete_async_signal_handler (async_signa
   (*async_handler_ptr) = NULL;
 }
 
-/* Is it necessary to call invoke_async_signal_handler? */
-static int
-check_async_ready (void)
+/* Create an asynchronous event handler, allocating memory for it.
+   Return a pointer to the newly created handler.  PROC is the
+   function to call with CLIENT_DATA argument whenever the handler is
+   invoked.  */
+async_event_handler *
+create_async_event_handler (async_event_handler_func *proc,
+			    gdb_client_data client_data)
+{
+  async_event_handler *h;
+
+  h = xmalloc (sizeof (*h));
+  h->ready = 0;
+  h->next_handler = NULL;
+  h->proc = proc;
+  h->client_data = client_data;
+  if (async_event_handler_list.first_handler == NULL)
+    async_event_handler_list.first_handler = h;
+  else
+    async_event_handler_list.last_handler->next_handler = h;
+  async_event_handler_list.last_handler = h;
+  return h;
+}
+
+/* Mark the handler (ASYNC_HANDLER_PTR) as ready.  This information
+   will be used by gdb_do_one_event.  The caller will be whoever
+   created the event source, and wants to signal that the event is
+   ready to be handled.  */
+void
+mark_async_event_handler (async_event_handler* async_handler_ptr)
+{
+  async_handler_ptr->ready = 1;
+}
+
+struct async_event_handler_data
+{
+  async_event_handler_func* proc;
+  gdb_client_data client_data;
+};
+
+static void
+invoke_async_event_handler (event_data data)
+{
+  struct async_event_handler_data *hdata = data.ptr;
+  async_event_handler_func* proc = hdata->proc;
+  gdb_client_data client_data = hdata->client_data;
+
+  xfree (hdata);
+  (*proc) (client_data);
+}
+
+/* Check if any asynchronous event handlers are ready, and queue
+   events in the ready queue for any that are.  */
+static void
+check_async_event_handlers (void)
 {
-  return async_handler_ready;
+  async_event_handler *async_handler_ptr;
+  struct async_event_handler_data *hdata;
+  struct gdb_event *event_ptr;
+  event_data data;
+
+  for (async_handler_ptr = async_event_handler_list.first_handler;
+       async_handler_ptr != NULL;
+       async_handler_ptr = async_handler_ptr->next_handler)
+    {
+      if (async_handler_ptr->ready)
+	{
+	  async_handler_ptr->ready = 0;
+
+	  hdata = xmalloc (sizeof (*hdata));
+
+	  hdata->proc = async_handler_ptr->proc;
+	  hdata->client_data = async_handler_ptr->client_data;
+
+	  data.ptr = hdata;
+
+	  event_ptr = create_event (invoke_async_event_handler, data);
+	  async_queue_event (event_ptr, TAIL);
+	}
+    }
+}
+
+/* Delete an asynchronous handler (ASYNC_HANDLER_PTR).
+   Free the space allocated for it.  */
+void
+delete_async_event_handler (async_event_handler** async_handler_ptr)
+{
+  async_event_handler *prev_ptr;
+
+  if (async_event_handler_list.first_handler == *async_handler_ptr)
+    {
+      async_event_handler_list.first_handler = (*async_handler_ptr)->next_handler;
+      if (async_event_handler_list.first_handler == NULL)
+	async_event_handler_list.last_handler = NULL;
+    }
+  else
+    {
+      prev_ptr = async_event_handler_list.first_handler;
+      while (prev_ptr && prev_ptr->next_handler != *async_handler_ptr)
+	prev_ptr = prev_ptr->next_handler;
+      prev_ptr->next_handler = (*async_handler_ptr)->next_handler;
+      if (async_event_handler_list.last_handler == (*async_handler_ptr))
+	async_event_handler_list.last_handler = prev_ptr;
+    }
+  xfree (*async_handler_ptr);
+  *async_handler_ptr = NULL;
 }
 
 /* Create a timer that will expire in MILLISECONDS from now. When the
@@ -1080,11 +1275,11 @@ delete_timer (int id)
 }
 
 /* When a timer event is put on the event queue, it will be handled by
-   this function.  Just call the assiciated procedure and delete the
-   timer event from the event queue. Repeat this for each timer that
-   has expired. */
+   this function.  Just call the associated procedure and delete the
+   timer event from the event queue.  Repeat this for each timer that
+   has expired.  */
 static void
-handle_timer_event (int dummy)
+handle_timer_event (event_data dummy)
 {
   struct timeval time_now;
   struct gdb_timer *timer_ptr, *saved_timer;
@@ -1150,7 +1345,7 @@ poll_timers (void)
 	{
 	  event_ptr = (gdb_event *) xmalloc (sizeof (gdb_event));
 	  event_ptr->proc = handle_timer_event;
-	  event_ptr->fd = timer_list.first_timer->timer_id;
+	  event_ptr->data.integer = timer_list.first_timer->timer_id;
 	  async_queue_event (event_ptr, TAIL);
 	}
 

Re: generic async event handlers in the event loop, for remote non-stop (was: generic `struct serial' interface pipe for remote non-stop)

[Eli Zaretskii]

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Fri, 24 Oct 2008 01:32:41 +0100
> 
> signal handlers should keep the current behaviour of having high
> priority in relation to normal events

I don't necessarily disagree, but can you explain why is that?

Re: generic async event handlers in the event loop, for remote non-stop (was: generic `struct serial' interface pipe for remote non-stop)

[Pedro Alves]

On Friday 24 October 2008 10:54:36, Eli Zaretskii wrote:
> > From: Pedro Alves <pedro@codesourcery.com>
> > Date: Fri, 24 Oct 2008 01:32:41 +0100
> > 
> > signal handlers should keep the current behaviour of having high
> > priority in relation to normal events
> 
> I don't necessarily disagree, but can you explain why is that?

Sure!  But, do you see a case to change that established behaviour?

We give them high priority (we delay signals until we get to the
event loop, but handle them immediatelly once we get there, we don't
delay them further) .  That isn't appropriate for my event sources,
as I explained before, so we can't have it both ways in a
single mechanism.

If we change signal handlers to have as much priority as normal event
sources, then we risk the case were a signal handler is delayed further
(and indefinitely, until some another signal arrives) due to entering a
blocking call (like poll/select/wait etc.), due to handling some other
event before the signal's turn in the event loop arrives.

This current behaviour isn't super perfect, as the signal could
be marked just before entering poll/select, but, that's the current
behaviour, and should be fixed separately when there's a need.  Lowering
the signal handlers priority is a step in the wrong direction, IMHO.

-- 
Pedro Alves

Re: generic async event handlers in the event loop, for remote non-stop (was: generic `struct serial' interface pipe for remote non-stop)

[Eli Zaretskii]

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Fri, 24 Oct 2008 15:12:54 +0100
> 
> On Friday 24 October 2008 10:54:36, Eli Zaretskii wrote:
> > > From: Pedro Alves <pedro@codesourcery.com>
> > > Date: Fri, 24 Oct 2008 01:32:41 +0100
> > > 
> > > signal handlers should keep the current behaviour of having high
> > > priority in relation to normal events
> > 
> > I don't necessarily disagree, but can you explain why is that?
> 
> Sure!  But, do you see a case to change that established behaviour?

No, not after your explanation (mostly that they are already
delayed at that point).

Thanks.

Re: generic async event handlers in the event loop, for remote non-stop (was: generic `struct serial' interface pipe for remote non-stop)

[Pedro Alves]

On Friday 24 October 2008 17:39:48, Eli Zaretskii wrote:
> No, not after your explanation (mostly that they are already
> delayed at that point).
> 
> Thanks.

Thank you!  I've now checked the patch in.

-- 
Pedro Alves

Re: generic async event handlers in the event loop, for remote non-stop (was: generic `struct serial' interface pipe for remote non-stop)

[Pedro Alves]

On Friday 24 October 2008 20:36:34, Pedro Alves wrote:
> Thank you!  I've now checked the patch in.

Forgot to mention --- I tested this on both native and local gdbserver on 
x86-64-unknown-linux-gnu (both -m64/-m32), and also gave it a spin on native
mingw32 in our internal tree.

Let me know if you spot any weirdness related to this.

-- 
Pedro Alves