This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RFA: properly handle the GIL


Python has a global interpreter lock (the "GIL") which is used to
mediate access to the Python core when there are multiple threads.
This is needed because the core is not thread-safe.

Currently, gdb acquires the GIL at startup and does not release it,
even when waiting for user input.  This means that if a Python script
running in gdb creates a new thread, no work will happen much of the
time.

This patch changes gdb to only acquire the GIL before running Python
code.  This is now a general rule for hacking on the Python
integration: if the call chain originates in gdb, and enters Python,
then the GIL must first be acquired.  Currently only the "python"
command is affected, but there are more instances on the python
branch.

Note the use of WITH_THREAD.  This is defined via Python.h -- bogusly,
IMO, but it appears to be what Python extensions are supposed to rely
on.

Built and regtested on x86-64 (compile farm).

Note that the compile farm is running Python 2.4.  I have 2.5 here,
and so I have built it against both versions.

I have not built a threadless Python, but you can see the intended
direction in the python-internal.h patch; I did temporarily #undef
WITH_THREAD to ensure that these macros compile correctly.

Ok?

Tom

2008-11-19  Tom Tromey  <tromey@redhat.com>

	* python/python-internal.h (PyGILState_Ensure): New define.
	(PyGILState_Release): Likewise.
	(PyEval_InitThreads): Likewise.
	(PyThreadState_Swap): Likewise.
	(PyEval_InitThreads): Likewise.
	* python/python.c (_initialize_python): Initialize threads.
	Release GIL.
	(eval_python_from_control_command): Acquire GIL.
	(python_command): Likewise.
	* python/python-internal.h (make_cleanup_py_restore_gil):
	Declare.
	* python/python-utils.c (py_gil_restore): New function.
	(make_cleanup_py_restore_gil): Likewise.

diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 72f7a5f..dbc0a53 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -43,6 +43,17 @@ typedef Py_intptr_t Py_ssize_t;
 #error "Unable to find usable Python.h"
 #endif
 
+/* If Python.h does not define WITH_THREAD, then the various
+   GIL-related functions will not be defined.  However,
+   PyGILState_STATE will be.  */
+#ifndef WITH_THREAD
+#define PyGILState_Ensure() ((PyGILState_STATE) 0)
+#define PyGILState_Release(ARG) (ARG)
+#define PyEval_InitThreads() 0
+#define PyThreadState_Swap(ARG) (ARG)
+#define PyEval_InitThreads() 0
+#endif
+
 struct value;
 
 extern PyObject *gdb_module;
@@ -57,6 +68,7 @@ struct value *convert_value_from_python (PyObject *obj);
 void gdbpy_initialize_values (void);
 
 struct cleanup *make_cleanup_py_decref (PyObject *py);
+struct cleanup *make_cleanup_py_restore_gil (PyGILState_STATE *state);
 
 /* Use this after a TRY_EXCEPT to throw the appropriate Python
    exception.  */
diff --git a/gdb/python/python-utils.c b/gdb/python/python-utils.c
index 912845f..8db81ec 100644
--- a/gdb/python/python-utils.c
+++ b/gdb/python/python-utils.c
@@ -46,6 +46,23 @@ make_cleanup_py_decref (PyObject *py)
   return make_cleanup (py_decref, (void *) py);
 }
 
+/* A cleanup function to restore the thread state.  */
+
+static void
+py_gil_restore (void *p)
+{
+  PyGILState_STATE *state = p;
+  PyGILState_Release (*state);
+}
+
+/* Return a new cleanup which will restore the Python GIL state.  */
+
+struct cleanup *
+make_cleanup_py_restore_gil (PyGILState_STATE *state)
+{
+  return make_cleanup (py_gil_restore, state);
+}
+
 /* Converts a Python 8-bit string to a unicode string object.  Assumes the
    8-bit string is in the host charset.  If an error occurs during conversion,
    returns NULL with a python exception set.
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 77d8774..50277d4 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -103,10 +103,15 @@ void
 eval_python_from_control_command (struct command_line *cmd)
 {
   char *script;
+  struct cleanup *cleanup;
+  PyGILState_STATE state;
 
   if (cmd->body_count != 1)
     error (_("Invalid \"python\" block structure."));
 
+  state = PyGILState_Ensure ();
+  cleanup = make_cleanup_py_restore_gil (&state);
+
   script = compute_python_string (cmd->body_list[0]);
   PyRun_SimpleString (script);
   xfree (script);
@@ -115,6 +120,8 @@ eval_python_from_control_command (struct command_line *cmd)
       gdbpy_print_stack ();
       error (_("error while executing Python code"));
     }
+
+  do_cleanups (cleanup);
 }
 
 /* Implementation of the gdb "python" command.  */
@@ -122,6 +129,12 @@ eval_python_from_control_command (struct command_line *cmd)
 static void
 python_command (char *arg, int from_tty)
 {
+  struct cleanup *cleanup;
+  PyGILState_STATE state;
+
+  state = PyGILState_Ensure ();
+  cleanup = make_cleanup_py_restore_gil (&state);
+
   while (arg && *arg && isspace (*arg))
     ++arg;
   if (arg && *arg)
@@ -136,10 +149,11 @@ python_command (char *arg, int from_tty)
   else
     {
       struct command_line *l = get_command_line (python_control, "");
-      struct cleanup *cleanups = make_cleanup_free_command_lines (&l);
+      make_cleanup_free_command_lines (&l);
       execute_control_command_untraced (l);
-      do_cleanups (cleanups);
     }
+
+  do_cleanups (cleanup);
 }
 
 
@@ -392,6 +406,7 @@ Enables or disables printing of Python stack traces."),
 
 #ifdef HAVE_PYTHON
   Py_Initialize ();
+  PyEval_InitThreads ();
 
   gdb_module = Py_InitModule ("gdb", GdbMethods);
 
@@ -429,5 +444,10 @@ class GdbOutputFile:\n\
 sys.stderr = GdbOutputFile()\n\
 sys.stdout = GdbOutputFile()\n\
 ");
+
+  /* Release the GIL while gdb runs.  */
+  PyThreadState_Swap (NULL);
+  PyEval_ReleaseLock ();
+
 #endif /* HAVE_PYTHON */
 }


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]