[patch] Implement post_event for Python scripts.

[patch] Implement post_event for Python scripts.

[Phil Muldoon]

This patch (ported from Archer) allows Python scripts to inject events
directly into GDB's event queue.  Each object inserted into the queue
is executed sequentially, but no guarantee is given when these events
will be executed (this is down to when GDB decides to process events
in the queue). This API allows multi-threaded Python scripts to
interact with GDB in a thread-safe manner (assuming it uses post-event
to execute Python code).

Comments?

Tested on x8664 with no regressions.

Cheers,

Phil

ChangeLog

2010-07-20  Tom Tromey  <tromey@redhat.com>
            Phil Muldoon  <pmuldoon@redhat.com>

	* python/python.c (gdbpy_run_events): New function.
	(gdbpy_post_event): Likewise.
	(gdbpy_initialize_events): Likewise.
	(_initialize_python): Call gdbpy_initialize_events.


Documentation ChangeLog:

2010-07-20  Tom Tromey  <tromey@redhat.com>

	* gdb.texinfo (Basic Python): Describe post_event API.

Testsuite ChangeLog:

2010-07-20  Phil Muldoon  <pmuldoon@redhat.com>

	* gdb.python/python.exp (gdb_py_test_multiple): Add gdb.post_event
	tests.

--

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index be6cd3d..d47bcbe 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -20490,6 +20490,21 @@ compute values, for example, it is the only way to get the value of a
 convenience variable (@pxref{Convenience Vars}) as a @code{gdb.Value}.
 @end defun
 
+@findex gdb.post_event
+@defun post_event event
+Put @var{event}, a callable object taking no arguments, into
+@value{GDBN}'s internal event queue.  This callable will be invoked at
+some later point, during @value{GDBN}'s event processing.  Events
+posted using @code{post_event} will be run in the order in which they
+were posted; however, there is no way to know when they will be
+processed relative to other events inside @value{GDBN}.
+
+@value{GDBN} is not thread-safe.  If your Python program uses multiple
+threads, you must be careful to only call @value{GDBN}-specific
+functions in the main @value{GDBN} thread.  @code{post_event} ensures
+this.
+@end defun
+
 @findex gdb.write
 @defun write string
 Print a string to @value{GDBN}'s paginated standard output stream.
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 6680126..866eda5 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -28,6 +28,7 @@
 #include "value.h"
 #include "language.h"
 #include "exceptions.h"
+#include "event-loop.h"
 
 #include <ctype.h>
 
@@ -453,6 +454,113 @@ source_python_script (FILE *stream, const char *file)
 
 \f
 
+/* Posting and handling events.  */
+
+/* A single event.  */
+struct gdbpy_event
+{
+  /* The Python event.  This is just a callable object.  */
+  PyObject *event;
+  /* The next event.  */
+  struct gdbpy_event *next;
+};
+
+/* All pending events.  */
+static struct gdbpy_event *gdbpy_event_list;
+/* The final link of the event list.  */
+static struct gdbpy_event **gdbpy_event_list_end;
+
+/* We use a file handler, and not an async handler, so that we can
+   wake up the main thread even when it is blocked in poll().  */
+static int gdbpy_event_fds[2];
+
+/* The file handler callback.  This reads from the internal pipe, and
+   then processes the Python event queue.  This will always be run in
+   the main gdb thread.  */
+static void
+gdbpy_run_events (int err, gdb_client_data ignore)
+{
+  struct cleanup *cleanup;
+  char buffer[100];
+  int r;
+
+  cleanup = ensure_python_env (get_current_arch (), current_language);
+
+  /* Just read whatever is available on the fd.  It is relatively
+     harmless if there are any bytes left over.  */
+  r = read (gdbpy_event_fds[0], buffer, sizeof (buffer));
+
+  while (gdbpy_event_list)
+    {
+      /* Dispatching the event might push a new element onto the event
+	 loop, so we update here "atomically enough".  */
+      struct gdbpy_event *item = gdbpy_event_list;
+      gdbpy_event_list = gdbpy_event_list->next;
+      if (gdbpy_event_list == NULL)
+	gdbpy_event_list_end = &gdbpy_event_list;
+
+      /* Ignore errors.  */
+      PyObject_CallObject (item->event, NULL);
+
+      Py_DECREF (item->event);
+      xfree (item);
+    }
+
+  do_cleanups (cleanup);
+}
+
+/* Submit an event to the gdb thread.  */
+static PyObject *
+gdbpy_post_event (PyObject *self, PyObject *args)
+{
+  struct gdbpy_event *event;
+  PyObject *func;
+  int wakeup;
+
+  if (!PyArg_ParseTuple (args, "O", &func))
+    return NULL;
+
+  if (!PyCallable_Check (func))
+    {
+      PyErr_SetString (PyExc_RuntimeError, 
+		       _("Posted event is not callable"));
+      return NULL;
+    }
+
+  Py_INCREF (func);
+
+  /* From here until the end of the function, we have the GIL, so we
+     can operate on our global data structures without worrying.  */
+  wakeup = gdbpy_event_list == NULL;
+
+  event = XNEW (struct gdbpy_event);
+  event->event = func;
+  event->next = NULL;
+  *gdbpy_event_list_end = event;
+  gdbpy_event_list_end = &event->next;
+
+  /* Wake up gdb when needed.  */
+  if (wakeup)
+    {
+      char c = 'q';		/* Anything. */
+      if (write (gdbpy_event_fds[1], &c, 1) != 1)
+        return PyErr_SetFromErrno (PyExc_IOError);
+    }
+
+  Py_RETURN_NONE;
+}
+
+/* Initialize the Python event handler.  */
+static void
+gdbpy_initialize_events (void)
+{
+  if (!pipe (gdbpy_event_fds))
+    {
+      gdbpy_event_list_end = &gdbpy_event_list;
+      add_file_handler (gdbpy_event_fds[0], gdbpy_run_events, NULL);
+    }
+}
+
 /* Printing.  */
 
 /* A python function to write a single string using gdb's filtered
@@ -759,6 +867,7 @@ Enables or disables printing of Python stack traces."),
   gdbpy_initialize_lazy_string ();
   gdbpy_initialize_thread ();
   gdbpy_initialize_inferior ();
+  gdbpy_initialize_events ();
 
   PyRun_SimpleString ("import gdb");
   PyRun_SimpleString ("gdb.pretty_printers = []");
@@ -869,6 +978,9 @@ a boolean indicating if name is a field of the current implied argument\n\
 Parse String as an expression, evaluate it, and return the result as a Value."
   },
 
+  { "post_event", gdbpy_post_event, METH_VARARGS,
+    "Post an event into gdb's event loop." },
+
   { "target_charset", gdbpy_target_charset, METH_NOARGS,
     "target_charset () -> string.\n\
 Return the name of the current target charset." },
diff --git a/gdb/testsuite/gdb.python/python.exp b/gdb/testsuite/gdb.python/python.exp
index d0e6c63..e9dabf6 100644
--- a/gdb/testsuite/gdb.python/python.exp
+++ b/gdb/testsuite/gdb.python/python.exp
@@ -87,3 +87,17 @@ gdb_test "python import itertools; print 'IMPOR'+'TED'" "IMPORTED" "pythonX.Y/li
 gdb_test_no_output \
     "python x = gdb.execute('printf \"%d\", 23', to_string = True)"
 gdb_test "python print x" "23"
+
+# Test post_event.
+gdb_py_test_multiple "post event insertion" \
+  "python" "" \
+  "someVal = 0" "" \
+  "class Foo():" "" \
+  "  def __call__(self):" "" \
+  "    global someVal" "" \
+  "    someVal += 1" "" \
+  "gdb.post_event(Foo())" "" \
+  "end" ""
+
+gdb_test "python print someVal" "1" "test post event execution"
+gdb_test "python gdb.post_event(str(1))" "RuntimeError: Posted event is not callable.*" "Test non callable class"

Re: [patch] Implement post_event for Python scripts.

[Tom Tromey]

>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:

Phil> +      /* Ignore errors.  */
Phil> +      PyObject_CallObject (item->event, NULL);

I think this has to explicitly clear the error, like:

if (PyObject_CallObject (item->event, NULL) == NULL)
  PyErr_Clear ();

The code parts are ok with this fix.

Tom

Re: [patch] Implement post_event for Python scripts.

[Joel Brobecker]

> +@findex gdb.post_event
> +@defun post_event event
> +Put @var{event}, a callable object taking no arguments, into
> +@value{GDBN}'s internal event queue.  This callable will be invoked at
> +some later point, during @value{GDBN}'s event processing.  Events
> +posted using @code{post_event} will be run in the order in which they
> +were posted; however, there is no way to know when they will be
> +processed relative to other events inside @value{GDBN}.

Would it be useful to provide an exemple of how this feature could
be used?  It does not seem obvious, and I am not sure without reading
the GDB Manual that users are familiar with GDB's internal even queue...

-- 
Joel

Re: [patch] Implement post_event for Python scripts.

[Phil Muldoon]

On 27/07/10 17:29, Joel Brobecker wrote:
>> +@findex gdb.post_event
>> +@defun post_event event
>> +Put @var{event}, a callable object taking no arguments, into
>> +@value{GDBN}'s internal event queue.  This callable will be invoked at
>> +some later point, during @value{GDBN}'s event processing.  Events
>> +posted using @code{post_event} will be run in the order in which they
>> +were posted; however, there is no way to know when they will be
>> +processed relative to other events inside @value{GDBN}.
> 
> Would it be useful to provide an exemple of how this feature could
> be used?  It does not seem obvious, and I am not sure without reading
> the GDB Manual that users are familiar with GDB's internal even queue...

Providing an example of a multi-threaded python script using
post_event would be too complex for the manual (I'm not sure that was
what you wanted anyway).  I provided a small example that shows usage.
Is this okay?

I've just regenerated the documentation patch for this reply.

Cheers,

Phil

--

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index be6cd3d..c7e1827 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -20490,6 +20490,37 @@ compute values, for example, it is the only way to get the value of a
 convenience variable (@pxref{Convenience Vars}) as a @code{gdb.Value}.
 @end defun
 
+@findex gdb.post_event
+@defun post_event event
+Put @var{event}, a callable object taking no arguments, into
+@value{GDBN}'s internal event queue.  This callable will be invoked at
+some later point, during @value{GDBN}'s event processing.  Events
+posted using @code{post_event} will be run in the order in which they
+were posted; however, there is no way to know when they will be
+processed relative to other events inside @value{GDBN}.
+
+@value{GDBN} is not thread-safe.  If your Python program uses multiple
+threads, you must be careful to only call @value{GDBN}-specific
+functions in the main @value{GDBN} thread.  @code{post_event} ensures
+this. For example:
+
+@smallexample
+(@value{GDBP}) python 
+>class Writer():
+>  def __init__(self, message):
+>       self.message = message;
+>  def __call__(self):
+>       print self.message  
+>
+>gdb.post_event(Writer("Hello"))
+>gdb.post_event(Writer("World"))
+>end
+(@value{GDBP}) Hello
+World
+@end smallexample
+@end defun
+
 @findex gdb.write
 @defun write string
 Print a string to @value{GDBN}'s paginated standard output stream.

Re: [patch] Implement post_event for Python scripts.

[Joel Brobecker]

> Providing an example of a multi-threaded python script using
> post_event would be too complex for the manual (I'm not sure that was
> what you wanted anyway).  I provided a small example that shows usage.
> Is this okay?

I'm just trying to figure out how this feature can be useful, and
in particular when the callbacks are triggered. Perhaps, rather than
an example, what we need is a section that describes the event loop
and when it is queried.  We can then make a reference to that section.
It would have been nice if we could have had a small example that
actually did something cool, but if the concepts are clear enough,
the user should be able to figure out what it can use it for.

-- 
Joel

Re: [patch] Implement post_event for Python scripts.

[Eli Zaretskii]

> Date: Wed, 28 Jul 2010 15:17:34 +0100
> From: Phil Muldoon <pmuldoon@redhat.com>
> CC: gdb-patches ml <gdb-patches@sourceware.org>
> 
> I've just regenerated the documentation patch for this reply.

Thanks.

> +this. For example:
       ^^
Two spaces, please.

Okay with that change.

Re: [patch] Implement post_event for Python scripts.

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

>> Providing an example of a multi-threaded python script using
>> post_event would be too complex for the manual (I'm not sure that was
>> what you wanted anyway).  I provided a small example that shows usage.
>> Is this okay?

Joel> I'm just trying to figure out how this feature can be useful, and
Joel> in particular when the callbacks are triggered. Perhaps, rather than
Joel> an example, what we need is a section that describes the event loop
Joel> and when it is queried.

For the purposes of post_event, I think it is probably sufficient to say
that the callback will be executed in the main thread "sometime".  I
don't think we can really provide anything more concrete than that
anyhow.

Tom

Re: [patch] Implement post_event for Python scripts.

[Phil Muldoon]

On 30/07/10 22:42, Tom Tromey wrote:
>>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:
> 
>>> Providing an example of a multi-threaded python script using
>>> post_event would be too complex for the manual (I'm not sure that was
>>> what you wanted anyway).  I provided a small example that shows usage.
>>> Is this okay?
> 
> Joel> I'm just trying to figure out how this feature can be useful, and
> Joel> in particular when the callbacks are triggered. Perhaps, rather than
> Joel> an example, what we need is a section that describes the event loop
> Joel> and when it is queried.


It's not useful to single-threaded scenarios (which I think are
going to be the vast number of applications of Python scripting).  For
multi-threaded scenarios (say a UI running on top of GDB that has its
own UI thread and event-loop) this is the only thread-safe way to
interact with GDB.  Describing the GDB even-loop would seem to be more
the province of an internals manual.  That being said, I take your point.


> Tom> For the purposes of post_event, I think it is probably sufficient to say
> Tom> that the callback will be executed in the main thread "sometime".  I
> Tom> don't think we can really provide anything more concrete than that
> Tom> anyhow.

I wrote up a multi-threaded example, but it is still a very basic
shell of an example.  To really show a useful  example it would require
loading an inferior, etc., and that would probably be too in-depth for
the manual

Cheers,

Phil

--


diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 03b59a3..77e21f2 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -20533,6 +20533,45 @@ compute values, for example, it is the only way to get the value of a
 convenience variable (@pxref{Convenience Vars}) as a @code{gdb.Value}.
 @end defun
 
+@findex gdb.post_event
+@defun post_event event
+Put @var{event}, a callable object taking no arguments, into
+@value{GDBN}'s internal event queue.  This callable will be invoked at
+some later point, during @value{GDBN}'s event processing.  Events
+posted using @code{post_event} will be run in the order in which they
+were posted; however, there is no way to know when they will be
+processed relative to other events inside @value{GDBN}.
+
+@value{GDBN} is not thread-safe.  If your Python program uses multiple
+threads, you must be careful to only call @value{GDBN}-specific
+functions in the main @value{GDBN} thread.  @code{post_event} ensures
+this. For example:
+
+@smallexample
+(@value{GDBP}) python
+>import threading
+>
+>class Writer():
+> def __init__(self, message):
+>        self.message = message;
+> def __call__(self):
+>        gdb.write(self.message)
+>
+>class MyThread1 (threading.Thread):
+> def run (self):
+>        gdb.post_event(Writer("Hello "))
+>
+>class MyThread2 (threading.Thread):
+> def run (self):
+>        gdb.post_event(Writer("World\n"))
+>
+>MyThread1().start()
+>MyThread2().start()
+>end
+(@value{GDBP}) Hello World
+@end smallexample
+@end defun
+
 @findex gdb.write
 @defun write string
 Print a string to @value{GDBN}'s paginated standard output stream.

Re: [patch] Implement post_event for Python scripts.

[Eli Zaretskii]

> Date: Tue, 03 Aug 2010 15:26:15 +0100
> From: Phil Muldoon <pmuldoon@redhat.com>
> CC: Joel Brobecker <brobecker@adacore.com>,        gdb-patches ml <gdb-patches@sourceware.org>
> 
> I wrote up a multi-threaded example, but it is still a very basic
> shell of an example.  To really show a useful  example it would require
> loading an inferior, etc., and that would probably be too in-depth for
> the manual
> 
> Cheers,
> 
> Phil
> 
> --
> 
> 
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 03b59a3..77e21f2 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo

OK.  Thanks.

> +this. For example:
       ^^
Two spaces.

Re: [patch] Implement post_event for Python scripts.

[Tom Tromey]

>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:

Phil> I wrote up a multi-threaded example, but it is still a very basic
Phil> shell of an example.  To really show a useful  example it would require
Phil> loading an inferior, etc., and that would probably be too in-depth for
Phil> the manual

This has been lingering for a little while.

Phil, please check in what you have.  If the documentation is still
insufficient, I think it is fine to fix it up in a separate patch.
Joel, could you comment on that when you find some time?

Tom

Re: [patch] Implement post_event for Python scripts.

[Joel Brobecker]

> Phil, please check in what you have.  If the documentation is still
> insufficient, I think it is fine to fix it up in a separate patch.
> Joel, could you comment on that when you find some time?

I think that the documentation is just as good as we can make it.
I was about to comment that the threaded exammple isn't bringing much
compared to the non-threaded one, but then I refrained, because the
new example is still fine, and a little diversity does not hurt :).

-- 
Joel

Re: [patch] Implement post_event for Python scripts.

[Phil Muldoon]

On 11/08/10 19:35, Tom Tromey wrote:
>>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
> 
> Phil> I wrote up a multi-threaded example, but it is still a very basic
> Phil> shell of an example.  To really show a useful  example it would require
> Phil> loading an inferior, etc., and that would probably be too in-depth for
> Phil> the manual
> 
> 
> Tom> Phil, please check in what you have.  

> Joel> I think that the documentation is just as good as we can make it.

> Eli> OK.  Thanks.
> Eli> 
> Eli> +this. For example:
> Eli>      ^^
> Eli> Two spaces.

So committed.  Thanks for all the help!

Cheers,

Phil

Re: [patch] Implement post_event for Python scripts.

[Pedro Alves]

On Tuesday 20 July 2010 19:53:36, Phil Muldoon wrote:

> 2010-08-11  Tom Tromey  <tromey@redhat.com>
>            Phil Muldoon  <pmuldoon@redhat.com>
>
>        * python/python.c (gdbpy_run_events): New function.
>        (gdbpy_post_event): Likewise.
>        (gdbpy_initialize_events): Likewise.
>        (_initialize_python): Call gdbpy_initialize_events.

It was brought to my attention that this unfortunatly breaks
Windows builds:

cc1: warnings being treated as errors
/scratch/sandra/trunk45/obj/gdb-src-2010.09-999999-arm-none-eabi-i686-mingw32/gdb/python/python.c: 
In function 'gdbpy_initialize_events':
/scratch/sandra/trunk45/obj/gdb-src-2010.09-999999-arm-none-eabi-i686-mingw32/gdb/python/python.c:675: 
error: implicit declaration of function 'pipe'

This is:

>   if (!pipe (gdbpy_event_fds))
>     {
>       gdbpy_event_list_end = &gdbpy_event_list;
>       add_file_handler (gdbpy_event_fds[0], gdbpy_run_events, NULL);
>     }

`pipe' doesn't exist on Windows.  There's a _pipe function, but it has a
different interface (takes two remote arguments).  See
<http://sourceware.org/ml/gdb-patches/2008-10/msg00418.html>
and look for "gdb_pipe".

The expedient way to fix this I guess would be to resurrect gdb_pipe
from that patch, so that we have a consistent way across hosts
to create a pipe.  

I do agree that using pipes on common code to wake up the event loop
is tackling at the wrong level (we should have an abstracted way to
do this, using pipes on posix hosts, events on Windows, etc.), but,
I'm happy with a simpler way for now.

I'm also not sure whether ...

> /* We use a file handler, and not an async handler, so that we can
>    wake up the main thread even when it is blocked in poll().  */
> static int gdbpy_event_fds[2];

... mingw-htep.c:gdb_select will be smart enough to apply the
proper select for that file descriptor, or if we need to bring
the whole patch in.

What do you think about this?

-- 
Pedro Alves

Re: [patch] Implement post_event for Python scripts.

[Daniel Jacobowitz]

On Wed, Aug 18, 2010 at 02:45:52PM +0100, Pedro Alves wrote:
> I'm also not sure whether ...
> 
> > /* We use a file handler, and not an async handler, so that we can
> >    wake up the main thread even when it is blocked in poll().  */
> > static int gdbpy_event_fds[2];
> 
> ... mingw-htep.c:gdb_select will be smart enough to apply the
> proper select for that file descriptor, or if we need to bring
> the whole patch in.

I think it'll work; it falls back to _get_osfhandle, So if you can
WaitForMultipleObjects on the Windows pipe handle, it'll do OK.  It
was just for some serial cases where you needed a separate handle that
we had problems.  OTOH if waiting for the pipe handle means you can
connect to it, or some other interpretation, we'll be back to the
beginning.  You could maybe adapt ser-pipe.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch] Implement post_event for Python scripts.

[Pedro Alves]

On Wednesday 18 August 2010 15:36:11, Daniel Jacobowitz wrote:
> On Wed, Aug 18, 2010 at 02:45:52PM +0100, Pedro Alves wrote:
> > I'm also not sure whether ...
> > 
> > > /* We use a file handler, and not an async handler, so that we can
> > >    wake up the main thread even when it is blocked in poll().  */
> > > static int gdbpy_event_fds[2];
> > 
> > ... mingw-htep.c:gdb_select will be smart enough to apply the
> > proper select for that file descriptor, or if we need to bring
> > the whole patch in.
> 
> I think it'll work; it falls back to _get_osfhandle, So if you can
> WaitForMultipleObjects on the Windows pipe handle, it'll do OK.

If I am to trust my 2008-self, if we simply fallback like that, it won't
work.  The file descriptors returned from `_pipe' are not waitable
on.  The handles behind them belong to an anonymous pipe, and
WaitForMultipleObjects doesn't work on those.  I'm just starting
to look into this, so I guess I'll know better once I actually
try it out.

> It was just for some serial cases where you needed a separate handle that
> we had problems.  

> OTOH if waiting for the pipe handle means you can
> connect to it, or some other interpretation, we'll be back to the
> beginning.  

Not sure I understood.

> You could maybe adapt ser-pipe.

If I'm understanding you here, that's what the patch I pointed at
did.  Here's the same thing refreshed to head.  The idea of the patch
originally was to use "serial_pipe" (working at the struct serial layer)
instead of gdb_pipe in common code.

I'll know more in a bit.  I have to relearn Windows.  :-)

-- 
Pedro Alves

2008-10-16  Pedro Alves  <pedro@codesourcery.com>

	* serial.h (gdb_pipe, serial_pipe): Declare.
	* serial.c (serial_interface_lookup): Take a const char pointer.
	(serial_fdopen): Rename to ...
	(serial_fdopen_ops): ... this.  Add an OPS parameter and use it.
	Call the OPS' fdopen function if there is one.
	(serial_fdopen): Rewrite as wrapper to serial_fdopen_ops.
	(serial_pipe): New.
	(struct serial_ops) <fdopen>: New field.

	* ser-mingw.c (free_pipe_state):
	(free_pipe_state): Close output on non-pex pipes.
	(pipe_windows_fdopen): New.
	(gdb_pipe): New.
	(_initialize_ser_windows): Register pipe_windows_fdopen.
	* ser-go32.c (gdb_pipe): New.
	* ser-pipe.c (pipe_close): Close file descriptor even if there's
	no state pointer.
	(pipe_ops): Delete.
	(gdb_pipe): New.

---
 gdb/ser-go32.c  |    7 ++++++
 gdb/ser-mingw.c |   41 +++++++++++++++++++++++++++++++++--
 gdb/ser-pipe.c  |   33 ++++++++++++++++++++++------
 gdb/serial.c    |   65 +++++++++++++++++++++++++++++++++++++++++++-------------
 gdb/serial.h    |   14 ++++++++++++
 5 files changed, 136 insertions(+), 24 deletions(-)

Index: src/gdb/serial.h
===================================================================
--- src.orig/gdb/serial.h	2010-06-16 10:58:26.000000000 +0100
+++ src/gdb/serial.h	2010-08-18 18:22:01.000000000 +0100
@@ -56,6 +56,19 @@ extern struct serial *serial_fdopen (con
 
 extern void serial_close (struct serial *scb);
 
+/* Create a pipe, and put the read end in files[0], and the write end
+   in filde[1].  Returns 0 for success, negative value for error (in
+   which case errno contains the error).  */
+
+extern int gdb_pipe (int fildes[2]);
+
+/* Create a pipe with each end wrapped in a `struct serial' interface.
+   Put the read end in scbs[0], and the write end in scbs[1].  Returns
+   0 for success, negative value for error (in which case errno
+   contains the error).  */
+
+extern int serial_pipe (struct serial *scbs[2]);
+
 /* Push out all buffers and destroy SCB without closing the device.  */
 
 extern void serial_un_fdopen (struct serial *scb);
@@ -222,6 +235,7 @@ struct serial_ops
     struct serial_ops *next;
     int (*open) (struct serial *, const char *name);
     void (*close) (struct serial *);
+    int (*fdopen) (struct serial *, int fd);
     int (*readchar) (struct serial *, int timeout);
     int (*write) (struct serial *, const char *str, int len);
     /* Discard pending output */
Index: src/gdb/serial.c
===================================================================
--- src.orig/gdb/serial.c	2010-06-16 11:15:34.000000000 +0100
+++ src/gdb/serial.c	2010-08-18 18:22:01.000000000 +0100
@@ -49,7 +49,7 @@ static struct serial *scb_base;
 static char *serial_logfile = NULL;
 static struct ui_file *serial_logfp = NULL;
 
-static struct serial_ops *serial_interface_lookup (char *);
+static struct serial_ops *serial_interface_lookup (const char *);
 static void serial_logchar (struct ui_file *stream, int ch_type, int ch, int timeout);
 static const char logbase_hex[] = "hex";
 static const char logbase_octal[] = "octal";
@@ -147,7 +147,7 @@ serial_log_command (const char *cmd)
 
 \f
 static struct serial_ops *
-serial_interface_lookup (char *name)
+serial_interface_lookup (const char *name)
 {
   struct serial_ops *ops;
 
@@ -255,22 +255,27 @@ serial_for_fd (int fd)
   return NULL;
 }
 
-struct serial *
-serial_fdopen (const int fd)
+/* Open a new serial stream using a file handle, using serial
+   interface ops OPS.  */
+
+static struct serial *
+serial_fdopen_ops (const int fd, struct serial_ops *ops)
 {
   struct serial *scb;
-  struct serial_ops *ops;
 
-  for (scb = scb_base; scb; scb = scb->next)
-    if (scb->fd == fd)
-      {
-	scb->refcnt++;
-	return scb;
-      }
+  scb = serial_for_fd (fd);
+  if (scb)
+    {
+      scb->refcnt++;
+      return scb;
+    }
 
-  ops = serial_interface_lookup ("terminal");
   if (!ops)
-    ops = serial_interface_lookup ("hardwire");
+    {
+      ops = serial_interface_lookup ("terminal");
+      if (!ops)
+ 	ops = serial_interface_lookup ("hardwire");
+    }
 
   if (!ops)
     return NULL;
@@ -282,8 +287,6 @@ serial_fdopen (const int fd)
   scb->bufcnt = 0;
   scb->bufp = scb->buf;
 
-  scb->fd = fd;
-
   scb->name = NULL;
   scb->next = scb_base;
   scb->refcnt = 1;
@@ -293,11 +296,22 @@ serial_fdopen (const int fd)
   scb->async_context = NULL;
   scb_base = scb;
 
+  if ((ops->fdopen) != NULL)
+    (*ops->fdopen) (scb, fd);
+  else
+    scb->fd = fd;
+
   last_serial_opened = scb;
 
   return scb;
 }
 
+struct serial *
+serial_fdopen (const int fd)
+{
+  return serial_fdopen_ops (fd, NULL);
+}
+
 static void
 do_serial_close (struct serial *scb, int really_close)
 {
@@ -582,6 +596,27 @@ serial_done_wait_handle (struct serial *
 }
 #endif
 
+int
+serial_pipe (struct serial *scbs[2])
+{
+  struct serial_ops *ops;
+  int fildes[2];
+
+  ops = serial_interface_lookup ("pipe");
+  if (!ops)
+    {
+      errno = ENOSYS;
+      return -1;
+    }
+
+  if (gdb_pipe (fildes) == -1)
+    return -1;
+
+  scbs[0] = serial_fdopen_ops (fildes[0], ops);
+  scbs[1] = serial_fdopen_ops (fildes[1], ops);
+  return 0;
+}
+
 #if 0
 /* The connect command is #if 0 because I hadn't thought of an elegant
    way to wait for I/O on two `struct serial *'s simultaneously.  Two
Index: src/gdb/ser-mingw.c
===================================================================
--- src.orig/gdb/ser-mingw.c	2010-06-16 11:15:34.000000000 +0100
+++ src/gdb/ser-mingw.c	2010-08-18 18:22:01.000000000 +0100
@@ -802,8 +802,12 @@ free_pipe_state (struct pipe_state *ps)
   if (ps->input)
     fclose (ps->input);
   if (ps->pex)
-    pex_free (ps->pex);
-  /* pex_free closes ps->output.  */
+    {
+      pex_free (ps->pex);
+      /* pex_free closes ps->output.  */
+    }
+  else if (ps->output)
+    fclose (ps->output);
 
   xfree (ps);
 
@@ -888,6 +892,30 @@ pipe_windows_open (struct serial *scb, c
   return -1;
 }
 
+static int
+pipe_windows_fdopen (struct serial *scb, int fd)
+{
+  struct pipe_state *ps;
+
+  ps = make_pipe_state ();
+
+  ps->input = fdopen (fd, "r+");
+  if (! ps->input)
+    goto fail;
+
+  ps->output = fdopen (fd, "r+");
+  if (! ps->output)
+    goto fail;
+
+  scb->fd = fd;
+  scb->state = (void *) ps;
+
+  return 0;
+
+ fail:
+  free_pipe_state (ps);
+  return -1;
+}
 
 static void
 pipe_windows_close (struct serial *scb)
@@ -992,6 +1020,14 @@ pipe_avail (struct serial *scb, int fd)
   return numBytes;
 }
 
+int
+gdb_pipe (int pdes[2])
+{
+  if (_pipe (pdes, 512, _O_BINARY | _O_NOINHERIT) == -1)
+    return -1;
+  return 0;
+}
+
 struct net_windows_state
 {
   struct ser_console_state base;
@@ -1230,6 +1266,7 @@ _initialize_ser_windows (void)
   ops->next = 0;
   ops->open = pipe_windows_open;
   ops->close = pipe_windows_close;
+  ops->fdopen = pipe_windows_fdopen;
   ops->readchar = ser_base_readchar;
   ops->write = ser_base_write;
   ops->flush_output = ser_base_flush_output;
Index: src/gdb/ser-go32.c
===================================================================
--- src.orig/gdb/ser-go32.c	2010-06-16 11:15:34.000000000 +0100
+++ src/gdb/ser-go32.c	2010-08-18 18:22:01.000000000 +0100
@@ -859,6 +859,13 @@ static struct serial_ops dos_ops =
   (void (*)(struct serial *, int))NULL	/* change into async mode */
 };
 
+int
+gdb_pipe (int pdes[2])
+{
+  /* No support for pipes.  */
+  errno = ENOSYS;
+  return -1;
+}
 
 static void
 dos_info (char *arg, int from_tty)
Index: src/gdb/ser-pipe.c
===================================================================
--- src.orig/gdb/ser-pipe.c	2010-06-16 12:36:45.000000000 +0100
+++ src/gdb/ser-pipe.c	2010-08-18 18:24:02.000000000 +0100
@@ -157,23 +157,42 @@ pipe_close (struct serial *scb)
 {
   struct pipe_state *state = scb->state;
 
+  close (scb->fd);
+  scb->fd = -1;
+
   if (state != NULL)
     {
-      int pid = state->pid;
-      close (scb->fd);
-      scb->fd = -1;
+      kill (state->pid, SIGTERM);
+      /* Might be useful to check that the child does die,
+	 and while we're waiting for it to die print any remaining
+	 stderr output.  */
+
       if (scb->error_fd != -1)
 	close (scb->error_fd);
       scb->error_fd = -1;
       xfree (state);
       scb->state = NULL;
-      kill (pid, SIGTERM);
-      /* Might be useful to check that the child does die,
-	 and while we're waiting for it to die print any remaining
-	 stderr output.  */
     }
 }
 
+int
+gdb_pipe (int pdes[2])
+{
+#if !HAVE_SOCKETPAIR
+  errno = ENOSYS;
+  return -1;
+#else
+
+  if (socketpair (AF_UNIX, SOCK_STREAM, 0, pdes) < 0)
+    return -1;
+
+  /* If we don't do this, GDB simply exits when the remote side
+     dies.  */
+  signal (SIGPIPE, SIG_IGN);
+  return 0;
+#endif
+}
+
 void
 _initialize_ser_pipe (void)
 {

Re: [patch] Implement post_event for Python scripts.

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

Pedro> It was brought to my attention that this unfortunatly breaks
Pedro> Windows builds:

Oops, sorry about that.

Pedro> The expedient way to fix this I guess would be to resurrect gdb_pipe
Pedro> from that patch, so that we have a consistent way across hosts
Pedro> to create a pipe.  

Pedro> I do agree that using pipes on common code to wake up the event loop
Pedro> is tackling at the wrong level (we should have an abstracted way to
Pedro> do this, using pipes on posix hosts, events on Windows, etc.), but,
Pedro> I'm happy with a simpler way for now.

Any way is ok by me, but the existing event code in mingw-hdep.c looks
pretty simple too.  I don't understand all of it, but I could try to
make a patch taking this direction if you want.  Just let me know.

Tom

Re: [patch] Implement post_event for Python scripts.

[Pedro Alves]

On Wednesday 18 August 2010 19:13:29, Tom Tromey wrote:

> Pedro> The expedient way to fix this I guess would be to resurrect gdb_pipe
> Pedro> from that patch, so that we have a consistent way across hosts
> Pedro> to create a pipe.  

Okay, I've confirmed that the patch below on top of yesterday's (which
adds serial_pipe), works, and the python.exp:post_event test passes on
Windows/mingw32.   Replacing pipe with gdb_pipe alone is not enough,
as indeed pipes are not waitable entities.  I guess I will apply this
and yesterday's tomorrow if nobody objects.

> Pedro> I do agree that using pipes on common code to wake up the event loop
> Pedro> is tackling at the wrong level (we should have an abstracted way to
> Pedro> do this, using pipes on posix hosts, events on Windows, etc.), but,
> Pedro> I'm happy with a simpler way for now.
> 
> Any way is ok by me, but the existing event code in mingw-hdep.c looks
> pretty simple too.  I don't understand all of it, but I could try to
> make a patch taking this direction if you want.  Just let me know.

The thing is that that code is for gdb_select, not the event loop
per-se.  Hacking gdb_select to break out when we want to
wake up the event loop doesn't look abstractly right to me.
IMO, we have the event loop implemented upside down.  We have it
heavilly file descriptor / poll / select oriented on all hosts, and
shoehorn Windows in by having gdb_select be able to poll on
more descriptor types than Windows itself allows.  IMO, it should be
the other way around --- the select/poll code should be moved to
somewhere host dependent (posix-hdep.c, for example), and the event
loop should be host-abstracted at the wait-for-one-event level.  The
Windows implementation would look similar to gdb_select, but not
constrained by the `select' API.

But, I don't claim to have the clearer vision here, and I'm
certainly open to ideas and patches.  :-)

-- 
Pedro Alves

2010-08-19  Pedro Alves  <pedro@codesourcery.com>

	* python/python.c: Include "serial.h".
	(gdbpy_event_fds): Change type to `struct serial *' a array from
	int array.
	(gdbpy_run_events): Change parameters.  Use serial_readchar in
	place of read.
	(gdbpy_post_event): Use serial_write in place of write.
	(gdbpy_initialize_events): Use serial_pipe instead of pipe, and
	serial_async in place of add_file_handler.

---
 gdb/python/python.c |   22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

Index: src/gdb/python/python.c
===================================================================
--- src.orig/gdb/python/python.c	2010-08-20 00:10:56.000000000 +0100
+++ src/gdb/python/python.c	2010-08-20 01:11:48.000000000 +0100
@@ -29,6 +29,7 @@
 #include "language.h"
 #include "exceptions.h"
 #include "event-loop.h"
+#include "serial.h"
 
 #include <ctype.h>
 
@@ -568,23 +569,25 @@ static struct gdbpy_event **gdbpy_event_
 
 /* We use a file handler, and not an async handler, so that we can
    wake up the main thread even when it is blocked in poll().  */
-static int gdbpy_event_fds[2];
+static struct serial *gdbpy_event_fds[2];
 
 /* The file handler callback.  This reads from the internal pipe, and
    then processes the Python event queue.  This will always be run in
    the main gdb thread.  */
+
 static void
-gdbpy_run_events (int err, gdb_client_data ignore)
+gdbpy_run_events (struct serial *scb, void *context)
 {
   struct cleanup *cleanup;
-  char buffer[100];
   int r;
 
   cleanup = ensure_python_env (get_current_arch (), current_language);
 
-  /* Just read whatever is available on the fd.  It is relatively
-     harmless if there are any bytes left over.  */
-  r = read (gdbpy_event_fds[0], buffer, sizeof (buffer));
+  /* Flush the fd.  Do this before flushing the events list, so that
+     any new event post afterwards is sure to re-awake the event
+     loop.  */
+  while (serial_readchar (gdbpy_event_fds[0], 0) >= 0)
+    ;
 
   while (gdbpy_event_list)
     {
@@ -640,7 +643,8 @@ gdbpy_post_event (PyObject *self, PyObje
   if (wakeup)
     {
       char c = 'q';		/* Anything. */
-      if (write (gdbpy_event_fds[1], &c, 1) != 1)
+
+      if (serial_write (gdbpy_event_fds[1], &c, 1))
         return PyErr_SetFromErrno (PyExc_IOError);
     }
 
@@ -651,10 +655,10 @@ gdbpy_post_event (PyObject *self, PyObje
 static void
 gdbpy_initialize_events (void)
 {
-  if (!pipe (gdbpy_event_fds))
+  if (serial_pipe (gdbpy_event_fds) == 0)
     {
       gdbpy_event_list_end = &gdbpy_event_list;
-      add_file_handler (gdbpy_event_fds[0], gdbpy_run_events, NULL);
+      serial_async (gdbpy_event_fds[0], gdbpy_run_events, NULL);
     }
 }