Skip to content

Commit

Permalink
pythongh-106883 Fix deadlock in threaded application
Browse files Browse the repository at this point in the history
When using threaded applications, there is a high risk of a deadlock in
the intepreter.
It's a lock ordering deadlock with HEAD_LOCK(&_PyRuntime); and the GIL.

By disabling the GC during the _PyThread_CurrentFrames() and
_PyThread_CurrentExceptions() calls fixes the issue.
  • Loading branch information
diegorusso committed Apr 5, 2024
1 parent 625b0f9 commit 20a2b04
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 13 deletions.
49 changes: 49 additions & 0 deletions Lib/test/test_sys.py
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,55 @@ def g456():
leave_g.set()
t.join()

@threading_helper.reap_threads
@threading_helper.requires_working_threading()
@support.requires_fork()
def test_current_frames_exceptions_deadlock(self):
"""
Try to reproduce the bug raised in GH-106883 and GH-116969.
"""
import threading
import time

class MockObject:
def __init__(self):
# Create some garbage
self._list = list(range(10000))
# Call the functions under test
self._trace = sys._current_frames()
self._exceptions = sys._current_exceptions()

def thread_function(num_objects):
obj = None
for _ in range(num_objects):
# The sleep is needed to have a syscall: in interrupts the
# current thread, releases the GIL and gives way to other
# threads to be executed. In this way there are more chances
# to reproduce the bug.
time.sleep(0)
obj = MockObject()

NUM_OBJECTS = 25
NUM_THREADS = 1000

# 60 seconds should be enough for the test to be executed: if it
# is more than 60 seconds it means that the process is in deadlock
# hence the test fails
TIMEOUT = 60

# Test the sys._current_frames and sys._current_exceptions calls
pid = os.fork()
if pid: # parent process
support.wait_process(pid, exitcode=0, timeout=TIMEOUT)
else: # child process
# Run the actual test in the forked process.
for i in range(NUM_THREADS):
thread = threading.Thread(
target=thread_function, args=(NUM_OBJECTS,)
)
thread.start()
os._exit(0)

@threading_helper.reap_threads
@threading_helper.requires_working_threading()
def test_current_exceptions(self):
Expand Down
27 changes: 14 additions & 13 deletions Python/pystate.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
/* Thread and interpreter state structures and their interfaces */

#include "Python.h"
#include "objimpl.h"
#include "pycore_ceval.h"
#include "pycore_code.h" // stats
#include "pycore_frame.h"
Expand Down Expand Up @@ -1389,10 +1388,6 @@ PyThreadState_Next(PyThreadState *tstate) {
PyObject *
_PyThread_CurrentFrames(void)
{
// Disable the GC as this can cause a deadlock the interpreter.
// See issues 116969 and 106883.
PyGC_Disable();

PyThreadState *tstate = _PyThreadState_GET();
if (_PySys_Audit(tstate, "sys._current_frames", NULL) < 0) {
return NULL;
Expand All @@ -1403,6 +1398,9 @@ _PyThread_CurrentFrames(void)
return NULL;
}

// gh-106883: Disable the GC as this can cause the interpreter to deadlock
int gc_was_enabled = PyGC_Disable();

/* for i in all interpreters:
* for t in all of i's thread states:
* if t's frame isn't NULL, map t's id to its frame
Expand Down Expand Up @@ -1446,19 +1444,17 @@ _PyThread_CurrentFrames(void)
done:
HEAD_UNLOCK(runtime);

// Once we release the runtime, the GC can be reenabled.
PyGC_Enable();
// Once the runtime is released, the GC can be reenabled.
if (gc_was_enabled) {
PyGC_Enable();
}

return result;
}

PyObject *
_PyThread_CurrentExceptions(void)
{
// Disable the GC as this can cause a deadlock the interpreter.
// See issues 116969 and 106883.
PyGC_Disable();

PyThreadState *tstate = _PyThreadState_GET();

_Py_EnsureTstateNotNULL(tstate);
Expand All @@ -1472,6 +1468,9 @@ _PyThread_CurrentExceptions(void)
return NULL;
}

// gh-106883: Disable the GC as this can cause the interpreter to deadlock
int gc_was_enabled = PyGC_Disable();

/* for i in all interpreters:
* for t in all of i's thread states:
* if t's frame isn't NULL, map t's id to its frame
Expand Down Expand Up @@ -1513,8 +1512,10 @@ _PyThread_CurrentExceptions(void)
done:
HEAD_UNLOCK(runtime);

// Once we release the runtime, the GC can be reenabled.
PyGC_Enable();
// Once the runtime is released, the GC can be reenabled.
if (gc_was_enabled) {
PyGC_Enable();
}

return result;
}
Expand Down

0 comments on commit 20a2b04

Please sign in to comment.