Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.11] gh-106883 Fix deadlock in threaded application #117332

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 74 additions & 0 deletions Lib/test/test_sys.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from test.support.script_helper import assert_python_ok, assert_python_failure
from test.support import threading_helper
from test.support import import_helper
from test.support import skip_if_sanitizer
import textwrap
import unittest
import warnings
Expand Down Expand Up @@ -471,6 +472,79 @@ def g456():
leave_g.set()
t.join()

@skip_if_sanitizer(memory=True, address=True, reason= "Test too slow "
"when the address sanitizer is enabled.")
@threading_helper.reap_threads
@threading_helper.requires_working_threading()
@support.requires_fork()
def test_current_frames_exceptions_deadlock(self):
"""
Reproduce the bug raised in GH-106883 and GH-116969.
"""
import threading
import time
import signal

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 __del__(self):
# The presence of the __del__ method causes the deadlock when
# there is one thread executing the _current_frames or
# _current_exceptions functions and the other thread is
# running the GC:
# thread 1 has the interpreter lock and it is trying to
# acquire the GIL; thread 2 holds the GIL but is trying to
# acquire the interpreter lock.
# When the GC is running and it finds that an
# object has the __del__ method, it needs to execute the
# Python code in it and it requires the GIL to execute it
# (which will never happen because it is held by another thread
# blocked on the acquisition of the interpreter lock)
pass

def thread_function(num_objects):
obj = None
for _ in range(num_objects):
obj = MockObject()

# The number of objects should be big enough to increase the
# chances to call the GC.
NUM_OBJECTS = 1000
NUM_THREADS = 10

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

# Test the sys._current_frames and sys._current_exceptions calls
pid = os.fork()
if pid: # parent process
try:
support.wait_process(pid, exitcode=0, timeout=TIMEOUT)
except KeyboardInterrupt:
# When pressing CTRL-C kill the deadlocked process
os.kill(pid, signal.SIGTERM)
raise
else: # child process
# Run the actual test in the forked process.
threads = []
for i in range(NUM_THREADS):
thread = threading.Thread(
target=thread_function, args=(NUM_OBJECTS,)
)
threads.append(thread)
thread.start()
for t in threads:
t.join()
os._exit(0)

@threading_helper.reap_threads
@threading_helper.requires_working_threading()
def test_current_exceptions(self):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Disable GC during the _PyThread_CurrentFrames() and _PyThread_CurrentExceptions() calls to avoid the interpreter to deadlock.
18 changes: 18 additions & 0 deletions Python/pystate.c
Original file line number Diff line number Diff line change
Expand Up @@ -1398,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 @@ -1440,6 +1443,12 @@ _PyThread_CurrentFrames(void)

done:
HEAD_UNLOCK(runtime);

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

return result;
}

Expand All @@ -1459,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 @@ -1499,6 +1511,12 @@ _PyThread_CurrentExceptions(void)

done:
HEAD_UNLOCK(runtime);

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

return result;
}

Expand Down
Loading