[v3] Release the GIL while running a gdb command or expression

Message ID 20181016214909.678-1-tom@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey Oct. 16, 2018, 9:49 p.m. UTC
  PR python/23615 points out that gdb.execute_gdb_command does not
release the Python GIL.  This means that, while the gdb command is
running, other Python threads do not run.

This patch solves the problem by introducing a new RAII class that can
be used to temporarily release and then re-acquire the GIL, then puts
this into the appropriate places in execute_gdb_command and
gdbpy_parse_and_eval.

This version of the patch includes a test case from Phil, and
addresses the comments made by Pedro.  Tested on x86-64 Fedora 28
natively, and also using the native-extended-gdbserver board.

Note that the test still has a race, though I haven't managed to
trigger it.  So I am still not sure it should go in.

gdb/ChangeLog
2018-09-07  Tom Tromey  <tom@tromey.com>

	PR python/23615:
	* python/python.c (execute_gdb_command): Use gdbpy_allow_threads.
	(gdbpy_parse_and_eval): Likewise.
	* python/python-internal.h (gdbpy_allow_threads): New class.

gdb/testsuite/ChangeLog
2018-10-10  Phil Muldoon  <pmuldoon@redhat.com>

	PR python/23615:
	* gdb.python/py-gil-mthread.c: New file.
	* gdb.python/py-gil-mthread.py: New file.
	* gdb.python/py-gil-mthread.exp: New file.
---
 gdb/ChangeLog                               |  7 ++
 gdb/python/python-internal.h                | 25 ++++++
 gdb/python/python.c                         |  3 +
 gdb/testsuite/ChangeLog                     |  7 ++
 gdb/testsuite/gdb.python/py-gil-mthread.c   | 30 +++++++
 gdb/testsuite/gdb.python/py-gil-mthread.exp | 88 +++++++++++++++++++++
 gdb/testsuite/gdb.python/py-gil-mthread.py  | 43 ++++++++++
 7 files changed, 203 insertions(+)
 create mode 100644 gdb/testsuite/gdb.python/py-gil-mthread.c
 create mode 100644 gdb/testsuite/gdb.python/py-gil-mthread.exp
 create mode 100644 gdb/testsuite/gdb.python/py-gil-mthread.py
  

Patch

diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 1812abb5b7..de84bf7461 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -627,6 +627,31 @@  class gdbpy_enter_varobj : public gdbpy_enter
 
 };
 
+/* The opposite of gdb_enter: this releases the GIL around a region,
+   allowing other Python threads to run.  No Python APIs may be used
+   while this is active.  */
+class gdbpy_allow_threads
+{
+public:
+
+  gdbpy_allow_threads ()
+    : m_save (PyEval_SaveThread ())
+  {
+    gdb_assert (m_save != nullptr);
+  }
+
+  ~gdbpy_allow_threads ()
+  {
+    PyEval_RestoreThread (m_save);
+  }
+
+  DISABLE_COPY_AND_ASSIGN (gdbpy_allow_threads);
+
+private:
+
+  PyThreadState *m_save;
+};
+
 extern struct gdbarch *python_gdbarch;
 extern const struct language_defn *python_language;
 
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 8fbce78469..5c5dc07849 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -585,6 +585,8 @@  execute_gdb_command (PyObject *self, PyObject *args, PyObject *kw)
 
   TRY
     {
+      gdbpy_allow_threads allow_threads;
+
       struct interp *interp;
 
       std::string arg_copy = arg;
@@ -908,6 +910,7 @@  gdbpy_parse_and_eval (PyObject *self, PyObject *args)
 
   TRY
     {
+      gdbpy_allow_threads allow_threads;
       result = parse_and_eval (expr_str);
     }
   CATCH (except, RETURN_MASK_ALL)
diff --git a/gdb/testsuite/gdb.python/py-gil-mthread.c b/gdb/testsuite/gdb.python/py-gil-mthread.c
new file mode 100644
index 0000000000..ba767154d5
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-gil-mthread.c
@@ -0,0 +1,30 @@ 
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2014-2018 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see  <http://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+#include <unistd.h>
+
+int
+main (void)
+{
+  int i;
+  for (i = 0; i < 10; i++)
+    {
+      sleep (1); /* break-here */
+      printf ("Sleeping %d\n", i);
+    }
+}
diff --git a/gdb/testsuite/gdb.python/py-gil-mthread.exp b/gdb/testsuite/gdb.python/py-gil-mthread.exp
new file mode 100644
index 0000000000..27946c10fa
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-gil-mthread.exp
@@ -0,0 +1,88 @@ 
+# Copyright (C) 2014-2018 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test that the Python GIL is released by gdb.execute.
+
+standard_testfile .c
+set executable $testfile
+
+if { [prepare_for_testing $testfile.exp $executable $srcfile] } {
+    return -1
+}
+
+# Skip all tests if Python scripting is not enabled.
+if { [skip_python_tests] } { continue }
+
+if [gdb_skip_stdio_test "py-gil-mthread.exp"] {
+    return
+}
+
+if ![runto_main] {
+    return -1
+}
+
+gdb_breakpoint $srcfile:[gdb_get_line_number "break-here"] temporary
+gdb_continue_to_breakpoint "break-here" ".* break-here .*"
+
+set remote_python_file [gdb_remote_download host \
+			    ${srcdir}/${subdir}/${testfile}.py]
+
+set test "response"
+set sleeping_last -1
+set hello_last 0
+set minimal 5
+gdb_test_multiple "source $remote_python_file" $test {
+    -re "Error: unable to start thread\r\n" {
+	fail $test
+    }
+    -re "source \[^\r\n\]*\r\n" {
+	exp_continue
+    }
+    -re "Hello \\( (\[0-9\]+) \\)\r\n" {
+	set n $expect_out(1,string)
+	if { $hello_last + 1 != $n } {
+	    fail $test
+	} else {
+	    set hello_last $n
+	    if { $sleeping_last >= $minimal && $hello_last >= $minimal } {
+		pass $test
+	    } else {
+		exp_continue
+	    }
+	}
+    }
+    -re ".*Inferior .* exited normally.\r\n" {
+	# If we saw a Hello during the run, then it worked.
+	if {$hello_last > 0} {
+	    pass $test
+	} else {
+	    fail $test
+	}
+    }
+    -i $inferior_spawn_id
+    -re "Sleeping (\[0-9\]+)\r\n" {
+	set n $expect_out(1,string)
+	if { $sleeping_last + 1 != $n } {
+	    fail $test
+	} else {
+	    set sleeping_last $n
+	    if { $sleeping_last >= $minimal && $hello_last >= $minimal } {
+		pass $test
+	    } else {
+		exp_continue
+	    }
+	}
+    }
+}
diff --git a/gdb/testsuite/gdb.python/py-gil-mthread.py b/gdb/testsuite/gdb.python/py-gil-mthread.py
new file mode 100644
index 0000000000..e25676e716
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-gil-mthread.py
@@ -0,0 +1,43 @@ 
+# Copyright (C) 2014-2018 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+try:
+   import thread
+except:
+   import _thread
+import time
+import gdb
+
+# Define a function for the thread.
+def print_thread_hello():
+   count = 0
+   while count < 10:
+      time.sleep(1)
+      count += 1
+      print ("Hello ( %d )" % count)
+
+# Create a thread and continue.
+try:
+   thread.start_new_thread (print_thread_hello, ())
+   gdb.execute ("continue")
+except:
+   try:
+      _thread.start_new_thread (print_thread_hello, ())
+      gdb.execute ("continue")
+   except:
+      print ("Error: unable to start thread")
+
+while 1:
+   pass