This is the mail archive of the archer@sourceware.org mailing list for the Archer 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]

[python] thread handling


Today I was playing with pygtk, and I tried to get the gtk main loop
to run in a separate thread from gdb.  This didn't work.

The problem was that gdb's main thread acquired the python "GIL"
(global interpreter lock) and did not let go while waiting for user
input.

This patch solves the problem by adding a new rule: gdb only acquires
the GIL when python code is about to be run.  This basically means
adding some code around the few places where gdb calls into python --
pretty-printing, the "python" command, and various callback functions.

I see one test suite regression, but I doubt it is from this patch.
I'll deal with that later.

Tom

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

	* varobj.c (varobj_get_display_hint): Acquire GIL.
	(update_dynamic_varobj_children): Likewise.
	(install_new_value): Likewise.
	(varobj_set_visualizer): Likewise.
	(free_variable): Likewise.
	(value_get_print_value): Likewise.
	* python/python-cmd.c (cmdpy_destroyer): Acquire GIL.
	(cmdpy_function): Likewise.
	(cmdpy_completer): Likewise.
	* python/python-breakpoint.c (gdbpy_breakpoint_created): Acquire
	GIL.
	(gdbpy_breakpoint_deleted): Likewise.
	* python/python.c (_initialize_python): Initialize threads.
	Release GIL.
	(eval_python_from_control_command): Acquire GIL.
	(python_command): Likewise.
	(run_python_script): Likewise.
	(source_python_script): Likewise.
	(gdbpy_new_objfile): Likewise.
	(apply_val_pretty_printer): Likewise.
	(apply_varobj_pretty_printer): 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-breakpoint.c b/gdb/python/python-breakpoint.c
index 9aa3546..0dfeb28 100644
--- a/gdb/python/python-breakpoint.c
+++ b/gdb/python/python-breakpoint.c
@@ -385,6 +385,7 @@ gdbpy_breakpoint_created (int num)
 {
   breakpoint_object *newbp;
   struct breakpoint *bp;
+  PyGILState_STATE state;
 
   if (num < 0)
     return;
@@ -409,6 +410,8 @@ gdbpy_breakpoint_created (int num)
 
   ++bppy_live;
 
+  state = PyGILState_Ensure ();
+
   if (bppy_pending_object)
     {
       newbp = bppy_pending_object;
@@ -439,6 +442,8 @@ gdbpy_breakpoint_created (int num)
 
   /* Just ignore errors here.  */
   PyErr_Clear ();
+
+  PyGILState_Release (state);
 }
 
 /* Callback that is used when a breakpoint is deleted.  This will
@@ -446,6 +451,9 @@ gdbpy_breakpoint_created (int num)
 static void
 gdbpy_breakpoint_deleted (int num)
 {
+  PyGILState_STATE state;
+
+  state = PyGILState_Ensure ();
   if (BPPY_VALID_P (num))
     {
       bppy_breakpoints[num]->bp = NULL;
@@ -453,6 +461,7 @@ gdbpy_breakpoint_deleted (int num)
       bppy_breakpoints[num] = NULL;
       --bppy_live;
     }
+  PyGILState_Release (state);
 }
 
 
diff --git a/gdb/python/python-cmd.c b/gdb/python/python-cmd.c
index 6c5b144..63b5432 100644
--- a/gdb/python/python-cmd.c
+++ b/gdb/python/python-cmd.c
@@ -76,21 +76,27 @@ cmdpy_dont_repeat (PyObject *self, PyObject *args)
   Py_RETURN_NONE;
 }
 
-
 
 
 /* Called if the gdb cmd_list_element is destroyed.  */
 static void
 cmdpy_destroyer (struct cmd_list_element *self, void *context)
 {
+  cmdpy_object *cmd;
+  PyGILState_STATE state;
+
+  state = PyGILState_Ensure ();
+
   /* Release our hold on the command object.  */
-  cmdpy_object *cmd = (cmdpy_object *) context;
+  cmd = (cmdpy_object *) context;
   cmd->command = NULL;
   Py_DECREF (cmd);
 
   /* We allocated the name and doc string.  */
   xfree (self->name);
   xfree (self->doc);
+
+  PyGILState_Release (state);
 }
 
 /* Called by gdb to invoke the command.  */
@@ -99,6 +105,11 @@ cmdpy_function (struct cmd_list_element *command, char *args, int from_tty)
 {
   cmdpy_object *obj = (cmdpy_object *) get_cmd_context (command);
   PyObject *argobj, *ttyobj, *result;
+  struct cleanup *cleanup;
+  PyGILState_STATE state;
+
+  state = PyGILState_Ensure ();
+  cleanup = make_cleanup_py_restore_gil (&state);
 
   if (! obj)
     error (_("Invalid invocation of Python command object."));
@@ -148,6 +159,7 @@ cmdpy_function (struct cmd_list_element *command, char *args, int from_tty)
 	}
     }
   Py_DECREF (result);
+  do_cleanups (cleanup);
 }
 
 /* Called by gdb for command completion.  */
@@ -155,8 +167,13 @@ static char **
 cmdpy_completer (struct cmd_list_element *command, char *text, char *word)
 {
   cmdpy_object *obj = (cmdpy_object *) get_cmd_context (command);
-  PyObject *textobj, *wordobj, *resultobj;
-  char **result;
+  PyObject *textobj, *wordobj, *resultobj = NULL;
+  char **result = NULL;
+  struct cleanup *cleanup;
+  PyGILState_STATE state;
+
+  state = PyGILState_Ensure ();
+  cleanup = make_cleanup_py_restore_gil (&state);
 
   if (! obj)
     error (_("Invalid invocation of Python command object."));
@@ -164,7 +181,7 @@ cmdpy_completer (struct cmd_list_element *command, char *text, char *word)
     {
       /* If there is no complete method, don't error -- instead, just
 	 say that there are no completions.  */
-      return NULL;
+      goto done;
     }
 
   textobj = PyString_FromString (text);
@@ -182,8 +199,9 @@ cmdpy_completer (struct cmd_list_element *command, char *text, char *word)
     {
       /* Just swallow errors here.  */
       PyErr_Clear ();
-      return NULL;
+      goto done;
     }
+  make_cleanup_py_decref (resultobj);
 
   result = NULL;
   if (PySequence_Check (resultobj))
@@ -220,7 +238,8 @@ cmdpy_completer (struct cmd_list_element *command, char *text, char *word)
 
  done:
 
-  Py_DECREF (resultobj);
+  do_cleanups (cleanup);
+
   return result;
 }
 
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 8f756c8..19f39cb 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -98,6 +98,7 @@ void gdbpy_initialize_objfile (void);
 void gdbpy_initialize_parameters (void);
 
 struct cleanup *make_cleanup_py_decref (PyObject *py);
+struct cleanup *make_cleanup_py_restore_gil (PyGILState_STATE *state);
 
 char *gdbpy_parse_command_name (char *text,
 				struct cmd_list_element ***base_list,
diff --git a/gdb/python/python-utils.c b/gdb/python/python-utils.c
index 2fdd2b7..c6cece7 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 5ac8230..1ff3c25 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);
 }
 
 
@@ -529,6 +543,10 @@ void
 run_python_script (int argc, char **argv)
 {
   FILE *input;
+  PyGILState_STATE state;
+
+  /* We never free this, since we plan to exit at the end.  */
+  state = PyGILState_Ensure ();
 
   running_python_script = 1;
   PySys_SetArgv (argc - 1, argv + 1);
@@ -546,8 +564,14 @@ run_python_script (int argc, char **argv)
 void
 source_python_script (FILE *stream, char *file)
 {
+  PyGILState_STATE state;
+
+  state = PyGILState_Ensure ();
+
   PyRun_SimpleFile (stream, file);
+
   fclose (stream);
+  PyGILState_Release (state);
 }
 
 
@@ -569,10 +593,13 @@ gdbpy_new_objfile (struct objfile *objfile)
   char *filename;
   int len;
   FILE *input;
+  PyGILState_STATE state;
 
   if (!gdbpy_auto_load || !objfile || !objfile->name)
     return;
 
+  state = PyGILState_Ensure ();
+
   gdbpy_current_objfile = objfile;
 
   realname = gdb_realpath (objfile->name);
@@ -595,6 +622,8 @@ gdbpy_new_objfile (struct objfile *objfile)
   xfree (realname);
   xfree (filename);
   gdbpy_current_objfile = NULL;
+
+  PyGILState_Release (state);
 }
 
 /* Return the current Objfile, or None if there isn't one.  */
@@ -984,11 +1013,15 @@ apply_val_pretty_printer (struct type *type, const gdb_byte *valaddr,
   char *hint;
   struct cleanup *cleanups;
   int result = 0;
+  PyGILState_STATE state;
+
+  state = PyGILState_Ensure ();
+  cleanups = make_cleanup_py_restore_gil (&state);
 
   /* Find the constructor.  */
   func = find_pretty_printer (type);
   if (! func)
-    return 0;
+    goto done;
 
   /* Instantiate the printer.  */
   value = value_from_contents_and_address (type, valaddr, embedded_offset,
@@ -999,10 +1032,10 @@ apply_val_pretty_printer (struct type *type, const gdb_byte *valaddr,
   if (!printer)
     {
       gdbpy_print_stack ();
-      return 0;
+      goto done;
     }
 
-  cleanups = make_cleanup_py_decref (printer);
+  make_cleanup_py_decref (printer);
   if (printer != Py_None)
     {
       print_string_repr (printer, stream, format, deref_ref,
@@ -1013,6 +1046,7 @@ apply_val_pretty_printer (struct type *type, const gdb_byte *valaddr,
       result = 1;
     }
 
+ done:
   do_cleanups (cleanups);
   return result;
 }
@@ -1029,8 +1063,14 @@ char *
 apply_varobj_pretty_printer (PyObject *printer_obj, struct value *value,
 			     struct value **replacement)
 {
+  char *result;
+  PyGILState_STATE state = PyGILState_Ensure ();
+
   *replacement = NULL;
-  return pretty_print_one_value (printer_obj, replacement);
+  result = pretty_print_one_value (printer_obj, replacement);
+  PyGILState_Release (state);
+
+  return result;
 }
 
 /* Find a pretty-printer object for the varobj module.  Returns a new
@@ -1207,6 +1247,7 @@ Enables or disables auto-loading of Python code when an object is opened."),
 
 #ifdef HAVE_PYTHON
   Py_Initialize ();
+  PyEval_InitThreads ();
 
   gdb_module = Py_InitModule ("gdb", GdbMethods);
 
@@ -1262,6 +1303,10 @@ sys.stderr = GdbOutputFile()\n\
 sys.stdout = GdbOutputFile()\n\
 ");
 
+  /* Release the GIL while gdb runs.  */
+  PyThreadState_Swap (NULL);
+  PyEval_ReleaseLock ();
+
 #endif /* HAVE_PYTHON */
 }
 
diff --git a/gdb/varobj.c b/gdb/varobj.c
index e8e627f..0c5bf6f 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -772,8 +772,10 @@ varobj_get_display_hint (struct varobj *var)
   char *result = NULL;
 
 #if HAVE_PYTHON
+  PyGILState_STATE state = PyGILState_Ensure ();
   if (var->pretty_printer)
     result = gdbpy_get_display_hint (var->pretty_printer);
+  PyGILState_Release (state);
 #endif
 
   return result;
@@ -828,10 +830,17 @@ update_dynamic_varobj_children (struct varobj *var,
   int i;
   int children_changed = 0;
   PyObject *printer = var->pretty_printer;
+  PyGILState_STATE state;
+
+  state = PyGILState_Ensure ();
+  back_to = make_cleanup_py_restore_gil (&state);
 
   *cchanged = 0;
   if (!PyObject_HasAttr (printer, gdbpy_children_cst))
-    return 0;
+    {
+      do_cleanups (back_to);
+      return 0;
+    }
 
   children = PyObject_CallMethodObjArgs (printer, gdbpy_children_cst,
 					 NULL);
@@ -842,7 +851,7 @@ update_dynamic_varobj_children (struct varobj *var,
       error ("Null value returned for children");
     }
 
-  back_to = make_cleanup_py_decref (children);
+  make_cleanup_py_decref (children);
 
   if (!PyIter_Check (children))
     error ("Returned value is not iterable");
@@ -1269,14 +1278,18 @@ install_new_value (struct varobj *var, struct value *value, int initial)
     }
 
 #if HAVE_PYTHON
-  if (var->pretty_printer)
-    {
-      Py_DECREF (var->pretty_printer);
-    }
-  if (value)
-    var->pretty_printer = instantiate_pretty_printer (var, value);
-  else
-    var->pretty_printer = NULL;
+  {
+    PyGILState_STATE state = PyGILState_Ensure ();
+    if (var->pretty_printer)
+      {
+	Py_DECREF (var->pretty_printer);
+      }
+    if (value)
+      var->pretty_printer = instantiate_pretty_printer (var, value);
+    else
+      var->pretty_printer = NULL;
+    PyGILState_Release (state);
+  }
 #endif
 
   /* Below, we'll be comparing string rendering of old and new
@@ -1417,11 +1430,18 @@ static void
 install_default_visualizer (struct varobj *var, struct type *type)
 {
 #if HAVE_PYTHON
+  struct cleanup *cleanup;
+  PyGILState_STATE state;
   PyObject *constructor = NULL;
 
+  state = PyGILState_Ensure ();
+  cleanup = make_cleanup_py_restore_gil (&state);
+
   if (type)
     constructor = gdbpy_get_varobj_pretty_printer (type);
   install_visualizer (var, constructor);
+
+  do_cleanups (cleanup);
 #else
   error ("Python support required");
 #endif
@@ -1433,11 +1453,15 @@ varobj_set_visualizer (struct varobj *var, const char *visualizer)
 #if HAVE_PYTHON
   PyObject *mainmod, *globals, *constructor;
   struct cleanup *back_to;
+  PyGILState_STATE state;
+
+  state = PyGILState_Ensure ();
+  back_to = make_cleanup_py_restore_gil (&state);
 
   mainmod = PyImport_AddModule ("__main__");
   globals = PyModule_GetDict (mainmod);
   Py_INCREF (globals);
-  back_to = make_cleanup_py_decref (globals);
+  make_cleanup_py_decref (globals);
 
   constructor = PyRun_String (visualizer, Py_eval_input, globals, globals);
 
@@ -1933,8 +1957,12 @@ free_variable (struct varobj *var)
     }
 
 #if HAVE_PYTHON
-  Py_XDECREF (var->constructor);
-  Py_XDECREF (var->pretty_printer);
+  {
+    PyGILState_STATE state = PyGILState_Ensure ();
+    Py_XDECREF (var->constructor);
+    Py_XDECREF (var->pretty_printer);
+    PyGILState_Release (state);
+  }
 #endif
 
   xfree (var->name);
@@ -2223,17 +2251,24 @@ value_get_print_value (struct value *value, enum varobj_display_formats format,
     return NULL;
 
 #if HAVE_PYTHON
-  if (value_formatter && PyObject_HasAttr (value_formatter,
-					   gdbpy_to_string_cst))
-    {
-      struct value *replacement;
-      thevalue = apply_varobj_pretty_printer (value_formatter, value,
-					      &replacement);
-      if (thevalue)
-	return thevalue;
-      if (replacement)
-	value = replacement;
-    }
+  {
+    PyGILState_STATE state = PyGILState_Ensure ();
+    if (value_formatter && PyObject_HasAttr (value_formatter,
+					     gdbpy_to_string_cst))
+      {
+	struct value *replacement;
+	thevalue = apply_varobj_pretty_printer (value_formatter, value,
+						&replacement);
+	if (thevalue)
+	  {
+	    PyGILState_Release (state);
+	    return thevalue;
+	  }
+	if (replacement)
+	  value = replacement;
+      }
+    PyGILState_Release (state);
+  }
 #endif
 
   stb = mem_fileopen ();


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