public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 1/2] gdb: move gdbpy_gil into python-internal.h
@ 2023-12-07 11:02 Alexandra Hájková
  2023-12-07 11:02 ` [PATCH 2/2] gdb/python: avoid use of _PyOS_ReadlineTState Alexandra Hájková
  2023-12-08 18:04 ` [PATCH 1/2] gdb: move gdbpy_gil into python-internal.h Tom Tromey
  0 siblings, 2 replies; 4+ messages in thread
From: Alexandra Hájková @ 2023-12-07 11:02 UTC (permalink / raw)
  To: gdb-patches

Move gdbpy_gil class into python-internal.h, the next
commit wants to make use of this class from a file other
than python.c.
---
 gdb/python/python-internal.h | 24 ++++++++++++++++++++++++
 gdb/python/python.c          | 24 ------------------------
 2 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 86eb5436f2a..14e15574685 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -772,6 +772,30 @@ class gdbpy_allow_threads
   PyThreadState *m_save;
 };
 
+/* A helper class to save and restore the GIL, but without touching
+   the other globals that are handled by gdbpy_enter.  */
+
+class gdbpy_gil
+{
+public:
+
+  gdbpy_gil ()
+    : m_state (PyGILState_Ensure ())
+  {
+  }
+
+  ~gdbpy_gil ()
+  {
+    PyGILState_Release (m_state);
+  }
+
+  DISABLE_COPY_AND_ASSIGN (gdbpy_gil);
+
+private:
+
+  PyGILState_STATE m_state;
+};
+
 /* Use this after a TRY_EXCEPT to throw the appropriate Python
    exception.  */
 #define GDB_PY_HANDLE_EXCEPTION(Exception)	\
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 8a36673a3e4..f9df57b9fd5 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -264,30 +264,6 @@ gdbpy_enter::finalize ()
   python_gdbarch = current_inferior ()->arch ();
 }
 
-/* A helper class to save and restore the GIL, but without touching
-   the other globals that are handled by gdbpy_enter.  */
-
-class gdbpy_gil
-{
-public:
-
-  gdbpy_gil ()
-    : m_state (PyGILState_Ensure ())
-  {
-  }
-
-  ~gdbpy_gil ()
-  {
-    PyGILState_Release (m_state);
-  }
-
-  DISABLE_COPY_AND_ASSIGN (gdbpy_gil);
-
-private:
-
-  PyGILState_STATE m_state;
-};
-
 /* Set the quit flag.  */
 
 static void
-- 
2.41.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2/2] gdb/python: avoid use of _PyOS_ReadlineTState
  2023-12-07 11:02 [PATCH 1/2] gdb: move gdbpy_gil into python-internal.h Alexandra Hájková
@ 2023-12-07 11:02 ` Alexandra Hájková
  2023-12-08 18:04   ` Tom Tromey
  2023-12-08 18:04 ` [PATCH 1/2] gdb: move gdbpy_gil into python-internal.h Tom Tromey
  1 sibling, 1 reply; 4+ messages in thread
From: Alexandra Hájková @ 2023-12-07 11:02 UTC (permalink / raw)
  To: gdb-patches

From: Andrew Burgess <aburgess@redhat.com>

In python/py-gdb-readline.c we make use of _PyOS_ReadlineTState,
however, this variable is no longer public in Python 3.13, and so GDB
no longer builds.

We are making use of _PyOS_ReadlineTState in order to re-acquire the
Python Global Interpreter Lock (GIL).  The _PyOS_ReadlineTState
variable is set in Python's outer readline code prior to calling the
user (GDB) supplied readline callback function, which for us is
gdbpy_readline_wrapper.  The gdbpy_readline_wrapper function is called
without the GIL held.

Instead of using _PyOS_ReadlineTState, I propose that we switch to
calling PyGILState_Ensure() and PyGILState_Release().  These functions
will acquire the GIL based on the current thread.  I think this should
be sufficient; I can't imagine why we'd be running
gdbpy_readline_wrapper on one thread on behalf of a different Python
thread.... that would be unexpected I think.
---
 gdb/python/py-gdb-readline.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/gdb/python/py-gdb-readline.c b/gdb/python/py-gdb-readline.c
index 124cec8055d..699d396dd19 100644
--- a/gdb/python/py-gdb-readline.c
+++ b/gdb/python/py-gdb-readline.c
@@ -56,13 +56,11 @@ gdbpy_readline_wrapper (FILE *sys_stdin, FILE *sys_stdout,
       if (except.reason == RETURN_QUIT)
 	return NULL;
 
-      /* The thread state is nulled during gdbpy_readline_wrapper,
-	 with the original value saved in the following undocumented
-	 variable (see Python's Parser/myreadline.c and
-	 Modules/readline.c).  */
-      PyEval_RestoreThread (_PyOS_ReadlineTState);
+
+      /* This readline callback is called without the GIL held.  */
+      gdbpy_gil gil;
+
       gdbpy_convert_exception (except);
-      PyEval_SaveThread ();
       return NULL;
     }
 
-- 
2.41.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] gdb: move gdbpy_gil into python-internal.h
  2023-12-07 11:02 [PATCH 1/2] gdb: move gdbpy_gil into python-internal.h Alexandra Hájková
  2023-12-07 11:02 ` [PATCH 2/2] gdb/python: avoid use of _PyOS_ReadlineTState Alexandra Hájková
@ 2023-12-08 18:04 ` Tom Tromey
  1 sibling, 0 replies; 4+ messages in thread
From: Tom Tromey @ 2023-12-08 18:04 UTC (permalink / raw)
  To: Alexandra Hájková; +Cc: gdb-patches

>>>>> "Alexandra" == Alexandra Hájková <ahajkova@redhat.com> writes:

Alexandra> Move gdbpy_gil class into python-internal.h, the next
Alexandra> commit wants to make use of this class from a file other
Alexandra> than python.c.

Thank you.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] gdb/python: avoid use of _PyOS_ReadlineTState
  2023-12-07 11:02 ` [PATCH 2/2] gdb/python: avoid use of _PyOS_ReadlineTState Alexandra Hájková
@ 2023-12-08 18:04   ` Tom Tromey
  0 siblings, 0 replies; 4+ messages in thread
From: Tom Tromey @ 2023-12-08 18:04 UTC (permalink / raw)
  To: Alexandra Hájková; +Cc: gdb-patches

>>>>> "Alexandra" == Alexandra Hájková <ahajkova@redhat.com> writes:

Alexandra> Instead of using _PyOS_ReadlineTState, I propose that we switch to
Alexandra> calling PyGILState_Ensure() and PyGILState_Release().  These functions
Alexandra> will acquire the GIL based on the current thread.  I think this should
Alexandra> be sufficient; I can't imagine why we'd be running
Alexandra> gdbpy_readline_wrapper on one thread on behalf of a different Python
Alexandra> thread.... that would be unexpected I think.

Makes sense to me.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-12-08 18:05 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-07 11:02 [PATCH 1/2] gdb: move gdbpy_gil into python-internal.h Alexandra Hájková
2023-12-07 11:02 ` [PATCH 2/2] gdb/python: avoid use of _PyOS_ReadlineTState Alexandra Hájková
2023-12-08 18:04   ` Tom Tromey
2023-12-08 18:04 ` [PATCH 1/2] gdb: move gdbpy_gil into python-internal.h Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).