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

diegorusso · 2024-03-28T17:37:06Z

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.

It has been suggested to disable GC during the
_PyThread_CurrentFrames() and _PyThread_CurrentExceptions() calls.

Issue: Deadlock in threaded application when using sys._current_frames #106883

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. It has been suggested to disable GC during the _PyThread_CurrentFrames() and _PyThread_CurrentExceptions() calls.

pablogsal · 2024-03-28T18:55:41Z

This will break users that have disabled the gc, we need to do the dance only if it has not been disabled

pablogsal · 2024-03-28T18:56:29Z

Also there are return paths for errors where this leaves the gc disabled

diegorusso · 2024-03-28T19:20:53Z

@pablogsal thanks for your comments! Of course this was more an "attempt" to check if the approach was in the right direction and to check if the C API I used are the correct ones. Next week I will produce a more complete patch.

colesbury

Yeah, I think this is the right general approach. I left some inline comments to expand on what @pablogsal wrote.

Python/pystate.c

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.

diegorusso · 2024-04-05T14:34:25Z

@pablogsal @colesbury I've updated the PR addressing your comments and adding a test to verify the patch. Please let me know what you think.

diegorusso · 2024-04-05T15:26:51Z

It is currently failing on the Address Sanitiser build, I'm investigating what the problem is.

diegorusso · 2024-04-09T13:01:45Z

@pablogsal @colesbury

I have pushed a new test for it. It is slower compare to the existent tests: it takes about 10 seconds on my machine (Parallels on MBP) when it succeeds. If it fails (it shouldn't) then it's the timeout of 40 seconds.
When used in the build with the address sanitizer enabled, it is skipped because it will be way slower.
My questions to you are: do we really need the test? if yes, can we mark it as slow?

Thoughts?

pablogsal · 2024-04-09T13:03:19Z

We have a thing to consider which is that 3.11.9 was the last bug fix release of 3.11 and that is already released. I could make an extra bug fix release to include a fix for this PR but technically this is too late. I will check with the other release managers to see what they t 8000 hink

diegorusso · 2024-04-09T15:12:01Z

Sure, no problem. Whatever the outcome is, it has been entertaining debugging it :)

diegorusso · 2024-05-27T22:19:56Z

I just want to give a gentle ping on this PR. What will be its fate? Merge or close? :) Joking apart, it would be nice to have a closure.

pablogsal · 2024-05-28T12:13:33Z

Unfortunately unless you want to argue that this is a security fix, we cannot backport bug fixes anymore to 3.11.

If this issue is something that's affecting a considerable number of users I am willing to reconsider, though. But in any case the release will be folded with the next security release of 3.11.

diegorusso · 2024-05-28T12:58:43Z

Hello, I don't think I can argue that this is a security fix (it isn't :) ) but also I left a comment on dask/distributed#8616 issue as they bumped into the same problem. I requested them to comment directly here: if they have found a way to workaround the issue, we can just close the PR otherwise we need to understand what the blockers are and what's the magnitude of the issue.

pablogsal · 2024-05-28T15:53:35Z

Let's check what the other RMs think just to get some consensus on this.

CC @ambv @Yhg1s @hugovk

tvalentyn · 2025-02-12T10:56:14Z

If this issue is something that's affecting a considerable number of users I am willing to reconsider, though. But in any case the release will be folded with the next security release of 3.11.

I work on a python library, and in the last ~2 months at least 3 of our customers have reached out for help debugging a deadlock situation. after much of very involved debugging, we attributed the issue to: #106883 .

pablogsal · 2025-03-10T21:50:09Z

CC @ambv @Yhg1s @hugovk

Ping @ambv @Yhg1s @hugovk

Yhg1s · 2025-03-10T23:45:56Z

3.11 is yours, Pablo, so it's really up to you. I recall working around this issue at Google, and while I think fixing it is worth while, as RM I'm not sure fixing it in 3.11, at this point is worth the risk...

That said, the risk is very small, and it is the case that we're pushing for more use of threads (and testing with threads) with free-threaded Python on the horizon, and it would definitely be nice if threads were less flaky in 3.11... But that's thinking more with my free-threaded contributor hat than my RM hat :P

6D40

pablogsal · 2025-03-11T15:29:04Z

I just wanted to double check with you and other RMs. But I am convinced to backport this

pablogsal · 2025-03-11T15:31:14Z

merged, this will go in the next security release

bedevere-bot · 2025-03-11T22:08:34Z

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot PPC64LE Fedora Stable 3.11 (tier-2) has failed when building commit 6b37486.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/933/builds/1172) and take a look at the build logs.
Check if the failure is related to this commit (6b37486) or if it is a false positive.
If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/933/builds/1172

Failed tests:

test_sys

Failed subtests:

test_current_frames_exceptions_deadlock - test.test_sys.SysModuleTest.test_current_frames_exceptions_deadlock

Summary of the results of the build (if available):

==

Click to see traceback logs

Traceback (most recent call last):
  File "/home/buildbot/buildarea/3.11.cstratak-fedora-stable-ppc64le/build/Lib/test/support/threading_helper.py", line 66, in decorator
    return func(*args)
           ^^^^^^^^^^^
  File "/home/buildbot/buildarea/3.11.cstratak-fedora-stable-ppc64le/build/Lib/test/test_sys.py", line 530, in test_current_frames_exceptions_deadlock
    support.wait_process(pid, exitcode=0, timeout=TIMEOUT)
  File "/home/buildbot/buildarea/3.11.cstratak-fedora-stable-ppc64le/build/Lib/test/support/__init__.py", line 1963, in wait_process
    raise AssertionError(f"process {pid} is still running "
AssertionError: process 2699152 is still running after 40.3 seconds


Traceback (most recent call last):
  File "/home/buildbot/buildarea/3.11.cstratak-fedora-stable-ppc64le/build/Lib/test/support/threading_helper.py", line 66, in decorator
    return func(*args)
           ^^^^^^^^^^^
  File "/home/buildbot/buildarea/3.11.cstratak-fedora-stable-ppc64le/build/Lib/test/test_sys.py", line 530, in test_current_frames_exceptions_deadlock
    support.wait_process(pid, exitcode=0, timeout=TIMEOUT)
  File "/home/buildbot/buildarea/3.11.cstratak-fedora-stable-ppc64le/build/Lib/test/support/__init__.py", line 1963, in wait_process
    raise AssertionError(f"process {pid} is still running "
AssertionError: process 2676978 is still running after 40.3 seconds

bedevere-bot · 2025-03-11T22:23:14Z

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot PPC64LE Fedora Stable Clang 3.11 (tier-3) has failed when building commit 6b37486.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/967/builds/1171) and take a look at the build logs.
Check if the failure is related to this commit (6b37486) or if it is a false positive.
If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/967/builds/1171

Failed tests:

test_sys

Failed subtests:

test_current_frames_exceptions_deadlock - test.test_sys.SysModuleTest.test_current_frames_exceptions_deadlock

Summary of the results of the build (if available):

==

Click to see traceback logs

Traceback (most recent call last):
  File "/home/buildbot/buildarea/3.11.cstratak-fedora-stable-ppc64le.clang/build/Lib/test/support/threading_helper.py", line 66, in decorator
    return func(*args)
           ^^^^^^^^^^^
  File "/home/buildbot/buildarea/3.11.cstratak-fedora-stable-ppc64le.clang/build/Lib/test/test_sys.py", line 530, in test_current_frames_exceptions_deadlock
    support.wait_process(pid, exitcode=0, timeout=TIMEOUT)
  File "/home/buildbot/buildarea/3.11.cstratak-fedora-stable-ppc64le.clang/build/Lib/test/support/__init__.py", line 1963, in wait_process
    raise AssertionError(f"process {pid} is still running "
AssertionError: process 2710960 is still running after 46.9 seconds


Traceback (most recent call last):
  File "/home/buildbot/buildarea/3.11.cstratak-fedora-stable-ppc64le.clang/build/Lib/test/support/threading_helper.py", line 66, in decorator
    return func(*args)
           ^^^^^^^^^^^
  File "/home/buildbot/buildarea/3.11.cstratak-fedora-stable-ppc64le.clang/build/Lib/test/test_sys.py", line 530, in test_current_frames_exceptions_deadlock
    support.wait_process(pid, exitcode=0, timeout=TIMEOUT)
  File "/home/buildbot/buildarea/3.11.cstratak-fedora-stable-ppc64le.clang/build/Lib/test/support/__init__.py", line 1963, in wait_process
    raise AssertionError(f"process {pid} is still running "
AssertionError: process 2732847 is still running after 40.3 seconds

pablogsal · 2025-03-11T23:17:25Z

Ugh, this is not good. @diegorusso do you mind checking? Otherwise we may need to revert :(

bedevere-bot · 2025-03-12T03:41:51Z

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot PPC64LE Fedora Stable Refleaks 3.11 (tier-2) has failed when building commit 6b37486.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/966/builds/843) and take a look at the build logs.
Check if the failure is related to this commit (6b37486) or if it is a false positive.
If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/966/builds/843

Failed tests:

test_sys

Failed subtests:

test_current_frames_exceptions_deadlock - test.test_sys.SysModuleTest.test_current_frames_exceptions_deadlock

Summary of the results of the build (if available):

==

Click to see traceback logs

Traceback (most recent call last):
  File "/home/buildbot/buildarea/3.11.cstratak-fedora-stable-ppc64le.refleak/build/Lib/test/support/threading_helper.py", line 66, in decorator
    return func(*args)
           ^^^^^^^^^^^
  File "/home/buildbot/buildarea/3.11.cstratak-fedora-stable-ppc64le.refleak/build/Lib/test/test_sys.py", line 530, in test_current_frames_exceptions_deadlock
    support.wait_process(pid, exitcode=0, timeout=TIMEOUT)
  File "/home/buildbot/buildarea/3.11.cstratak-fedora-stable-ppc64le.refleak/build/Lib/test/support/__init__.py", line 1963, in wait_process
    raise AssertionError(f"process {pid} is still running "
AssertionError: process 3111838 is still running after 40.3 seconds


Traceback (most recent call last):
  File "/home/buildbot/buildarea/3.11.cstratak-fedora-stable-ppc64le.refleak/build/Lib/test/support/threading_helper.py", line 66, in decorator
    return func(*args)
           ^^^^^^^^^^^
  File "/home/buildbot/buildarea/3.11.cstratak-fedora-stable-ppc64le.refleak/build/Lib/test/test_sys.py", line 530, in test_current_frames_exceptions_deadlock
    support.wait_process(pid, exitcode=0, timeout=TIMEOUT)
  File "/home/buildbot/buildarea/3.11.cstratak-fedora-stable-ppc64le.refleak/build/Lib/test/support/__init__.py", line 1963, in wait_process
    raise AssertionError(f"process {pid} is still running "
AssertionError: process 3114961 is still running after 40.3 seconds

bedevere-bot · 2025-03-12T05:16:07Z

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot PPC64LE RHEL8 3.11 (tier-2) has failed when building commit 6b37486.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/997/builds/1182) and take a look at the build logs.
Check if the failure is related to this commit (6b37486) or if it is a false positive.
If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/997/builds/1182

Failed tests:

test_sys

Failed subtests:

test_current_frames_exceptions_deadlock - test.test_sys.SysModuleTest.test_current_frames_exceptions_deadlock

Summary of the results of the build (if available):

==

Click to see traceback logs

Traceback (most recent call last):
  File "/home/buildbot/buildarea/3.11.cstratak-RHEL8-ppc64le/build/Lib/test/support/threading_helper.py", line 66, in decorator
    return func(*args)
           ^^^^^^^^^^^
  File "/home/buildbot/buildarea/3.11.cstratak-RHEL8-ppc64le/build/Lib/test/test_sys.py", line 530, in test_current_frames_exceptions_deadlock
    support.wait_process(pid, exitcode=0, timeout=TIMEOUT)
  File "/home/buildbot/buildarea/3.11.cstratak-RHEL8-ppc64le/build/Lib/test/support/__init__.py", line 1963, in wait_process
    raise AssertionError(f"process {pid} is still running "
AssertionError: process 3527836 is still running after 40.3 seconds


Traceback (most recent call last):
  File "/home/buildbot/buildarea/3.11.cstratak-RHEL8-ppc64le/build/Lib/test/support/threading_helper.py", line 66, in decorator
    return func(*args)
           ^^^^^^^^^^^
  File "/home/buildbot/buildarea/3.11.cstratak-RHEL8-ppc64le/build/Lib/test/test_sys.py", line 530, in test_current_frames_exceptions_deadlock
    support.wait_process(pid, exitcode=0, timeout=TIMEOUT)
  File "/home/buildbot/buildarea/3.11.cstratak-RHEL8-ppc64le/build/Lib/test/support/__init__.py", line 1963, in wait_process
    raise AssertionError(f"process {pid} is still running "
AssertionError: process 3511003 is still running after 40.5 seconds

bedevere-bot · 2025-03-12T07:18:56Z

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot PPC64LE RHEL8 Refleaks 3.11 (tier-2) has failed when building commit 6b37486.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/978/builds/854) and take a look at the build logs.
Check if the failure is related to this commit (6b37486) or if it is a false positive.
If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/978/builds/854

Failed tests:

test_sys

Failed subtests:

test_current_frames_exceptions_deadlock - test.test_sys.SysModuleTest.test_current_frames_exceptions_deadlock

Test leaking resources:

test_functools: memory blocks

Summary of the results of the build (if available):

==

Click to see traceback logs

Traceback (most recent call last):
  File "/home/buildbot/buildarea/3.11.cstratak-RHEL8-ppc64le.refleak/build/Lib/test/support/threading_helper.py", line 66, in decorator
    return func(*args)
           ^^^^^^^^^^^
  File "/home/buildbot/buildarea/3.11.cstratak-RHEL8-ppc64le.refleak/build/Lib/test/test_sys.py", line 530, in test_current_frames_exceptions_deadlock
    support.wait_process(pid, exitcode=0, timeout=TIMEOUT)
  File "/home/buildbot/buildarea/3.11.cstratak-RHEL8-ppc64le.refleak/build/Lib/test/support/__init__.py", line 1963, in wait_process
    raise AssertionError(f"process {pid} is still running "
AssertionError: process 3696628 is still running after 40.5 seconds


Traceback (most recent call last):
  File "/home/buildbot/buildarea/3.11.cstratak-RHEL8-ppc64le.refleak/build/Lib/test/support/threading_helper.py", line 66, in decorator
    return func(*args)
           ^^^^^^^^^^^
  File "/home/buildbot/buildarea/3.11.cstratak-RHEL8-ppc64le.refleak/build/Lib/test/test_sys.py", line 530, in test_current_frames_exceptions_deadlock
    support.wait_process(pid, exitcode=0, timeout=TIMEOUT)
  File "/home/buildbot/buildarea/3.11.cstratak-RHEL8-ppc64le.refleak/build/Lib/test/support/__init__.py", line 1963, in wait_process
    raise AssertionError(f"process {pid} is still running "
AssertionError: process 3726771 is still running after 40.3 seconds

diegorusso · 2025-03-12T10:44:18Z

Ugh, this is not good. @diegorusso do you mind checking? Otherwise we may need to revert :(

I'm looking into it.

pablogsal · 2025-03-12T22:58:05Z

Technically by our policy we will need to revert if this is not fixed in 24h but given that we don't have a release soon, let's wait until Friday EOD max

diegorusso · 2025-03-13T09:54:35Z

I plan to make a new PR today.

diegorusso · 2025-03-13T10:36:21Z

Upon an initial investigation, the culprit is the low timeout set in the test: increasing the timeout makes the test passing.

If we look at the builds related to this commit https://buildbot.python.org/#/changes/40264 we notice that all the failing ones related to this test have been built with --with-pydebug.
If we look at the file gcmodule.c (https://github.com/python/cpython/blob/6b37486184590d19c6f24e620545ec8f8f65e4c7/Modules/gcmodule.c) we see that in the file there are different code paths depending if the debug is enabled or not.
This has an impact on the execution time: with the debug enabled it takes more time execute the test case and it cannot finish by the timeout set of 40 seconds. The fact that happens on PPC64LE is because very likely these machines don't have a high clock speed and it might take some time to execute the test.
We can observe a similar failure on SPARCv9 Oracle Solaris as well https://buildbot.python.org/#/builders/988/builds/1187

On GitHub tests are passing because we build CPython without debug.

As a reference on my AArch64 Linux VM the execution time difference of test_sys between a build with and without debug is:

without debug: ~1.5 seconds
with debug: ~7 seconds

Another data point. On a software emulated environment:

without debug: ~17.3 seconds
with debug: ~68 seconds

There is about a 4x difference between the build with debug and without.

I suggest to increase the timeout to a level that it is enough for all the platforms, even the older ones.

diegorusso · 2025-03-13T11:30:41Z

PR has been raised: #131182

bedevere-app bot added the awaiting review label Mar 28, 2024

bedevere-app bot mentioned this pull request Mar 28, 2024

Deadlock in threaded application when using sys._current_frames #106883

Closed

colesbury self-requested a review March 28, 2024 19:35

colesbury reviewed Mar 28, 2024

View reviewed changes

Python/pystate.c Outdated Show resolved Hide resolved

Python/pystate.c Outdated Show resolved Hide resolved

Python/pystate.c Outdated 8000 Show resolved Hide resolved

Python/pystate.c Outdated Show resolved Hide resolved

diegorusso and others added 3 commits April 5, 2024 15:03

Merge branch 'python:3.11' into 3.11

625b0f9

📜🤖 Added by blurb_it.

6d25f11

diegorusso marked this pull request as draft April 5, 2024 17:13

bedevere-app bot removed the awaiting review label Apr 5, 2024

Improve test case

6a88da5

diegorusso marked this pull request as ready for review April 9, 2024 10:31

bedevere-app bot added the awaiting review label Apr 9, 2024

arhadthedev added the pending The issue will be closed if no feedback is provided label Apr 10, 2024

diegorusso mentioned this pull request Apr 12, 2024

Issue with Python 3.11 and dask[distributed] with high number of threads #116969

Closed

diegorusso mentioned this pull request May 28, 2024

Deadlock with python 3.11 dask/distributed#8616

Closed

diegorusso deleted the branch python:3.11 June 25, 2024 15:34

diegorusso closed this Jun 25, 2024

diegorusso deleted the 3.11 branch June 25, 2024 15:34

sbabashahi mentioned this pull request Feb 3, 2025

Deadlock in connection to DB newrelic/newrelic-python-agent#1299

Closed

diegorusso restored the 3.11 branch March 6, 2025 11:43

diegorusso reopened this Mar 6, 2025

pablogsal merged commit 6b37486 into python:3.11 Mar 11, 2025
21 of 22 checks passed

bedevere-app bot removed the awaiting review label Mar 11, 2025

diegorusso deleted the 3.11 branch March 11, 2025 16:01

AA-Turner removed the pending The issue will be closed if no feedback is provided label Apr 6, 2025

Uh oh!

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

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

Uh oh!

Conversation

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Uh oh!

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Uh oh!

Uh oh!

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Uh oh!

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Uh oh!

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!