From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19987 invoked by alias); 21 Nov 2008 01:38:50 -0000 Received: (qmail 19933 invoked by uid 22791); 21 Nov 2008 01:38:48 -0000 X-Spam-Check-By: sourceware.org Received: from mx2.redhat.com (HELO mx2.redhat.com) (66.187.237.31) by sourceware.org (qpsmtpd/0.31) with ESMTP; Fri, 21 Nov 2008 01:38:11 +0000 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id mAL1cA3N000979 for ; Thu, 20 Nov 2008 20:38:10 -0500 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx2.corp.redhat.com (8.13.1/8.13.1) with ESMTP id mAL1c9c1003436; Thu, 20 Nov 2008 20:38:09 -0500 Received: from opsy.redhat.com (vpn-13-160.rdu.redhat.com [10.11.13.160]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id mAL1c8Rl013232; Thu, 20 Nov 2008 20:38:08 -0500 Received: by opsy.redhat.com (Postfix, from userid 500) id 9A0913787AB; Thu, 20 Nov 2008 18:38:07 -0700 (MST) To: gdb-patches@sourceware.org Subject: RFA: properly handle the GIL From: Tom Tromey Reply-To: Tom Tromey X-Attribution: Tom Date: Fri, 21 Nov 2008 18:46:00 -0000 Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-11/txt/msg00575.txt.bz2 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 * 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 */ }