cpython icon indicating copy to clipboard operation
cpython copied to clipboard

gh-130522: Fix threading errors during garbage collection

Open TkTech opened this issue 9 months ago • 8 comments

This issue was introduced in 3.13 (5a1ecc8) and causes spurious errors when threads shutdown. This issue occurs on all platforms, and can be triggered by utilities like coverage.py's pytracer module or the PyCharm debugger (pydevd), which may keep references to internal threading objects around.

Issue was reproduced on Linux/OSX/Windows:

Exception ignored in: <function _DeleteDummyThreadOnDel.__del__ at 0x10a0811c0>
Traceback (most recent call last):
  File "/Library/Frameworks/Python.framework/Versions/3.13/lib/python3.13/threading.py", line 1383, in __del__
TypeError: 'NoneType' object does not support the context manager protocol
 Exception ignored in: <function _DeleteDummyThreadOnDel.__del__ at 0x7fedc963ff60>
Traceback (most recent call last):
  File "/opt/hostedtoolcache/Python/3.13.2/x64/lib/python3.13/threading.py", line 1383, in __del__
TypeError: 'NoneType' object does not support the context manager protocol
 Exception ignored in: <function _DeleteDummyThreadOnDel.__del__ at 0x000001A6B07BA840>
Traceback (most recent call last):
  File "C:\hostedtoolcache\windows\Python\3.13.2\x64\Lib\threading.py", line 1383, in __del__
TypeError: 'NoneType' object does not support the context manager protocol
  • Issue: gh-130522

TkTech avatar Mar 21 '25 07:03 TkTech

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

bedevere-app[bot] avatar Mar 21 '25 07:03 bedevere-app[bot]

Just chiming in... the following error was regularly displayed while using many addons in KODI: Exception ignored in: <function _DeleteDummyThreadOnDel.__del__ at 0xffff535a9b20> Traceback (most recent call last): File "/usr/lib/python3.13/threading.py", line 1383, in __del__ TypeError: 'NoneType' object does not support the context manager protocol

I can confirm after applying this PR that this is no longer happening.

mrfixit2001 avatar Mar 21 '25 12:03 mrfixit2001

Likewise, it's guaranteed to occur during the CI/CD tests in Chancy (https://github.com/TkTech/chancy/actions/runs/13992110608/job/39178225688) when run with coverage.py, and is fixed by this PR. However, I haven't been able to produce a minimal reproduction that would be suitable for a test case.

TkTech avatar Mar 21 '25 22:03 TkTech

I'm willing to do the digging, but it would be nice if one of you could at least isolate the bug down to a single case in your test suite so I have something to work off.

With the current master branch, this will trigger it 100% of the time when the SubInterepreterExecutor runs. The threaded executor will also occasionally error, but its failures are intermittent. It must be run with the coverage plugin or some other tool utilizing the trace module to trigger this issue.

git clone https://github.com/TkTech/chancy.git
cd chancy
docker-compose up (for a postgres server)
uv run pytest -s -vvvvv --cov=chancy --cov-report=xml tests/test_jobs.py

Thanks for looking into this.

TkTech avatar Mar 25 '25 15:03 TkTech

The garbage collector shouldn't be doing that; it destroys objects by clearing their reference cycles, not by setting them to None.

Correct, it's not the garbage collector per se. It's a different step in interpreter shutdown. Module-level globals are set to None here. (PEP 442 says this is no longer true since Python 3.4, but the code's still there?)

From what I can tell, this only comes up when multithreaded C extensions are active. That seems to be uvloop for the async-based reproducers

My repro in https://github.com/python/cpython/issues/130522#issuecomment-2698254391 does not require uvloop or any other third-party C extension. The only C modules in use are those in the standard library.

bdarnell avatar May 14 '25 18:05 bdarnell

Does Tornado use subinterpreters? That's what looks like the cause of the bug, at least with Chancy's case.

ZeroIntensity avatar May 14 '25 19:05 ZeroIntensity

No subinterpreters. There's not much going on in that test besides initialization of the asyncio event loop (which I think starts a thread? I can't remember if that's lazy or not) and populating a few threading.local variables.

bdarnell avatar May 14 '25 19:05 bdarnell

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

bedevere-app[bot] avatar Jun 27 '25 23:06 bedevere-app[bot]

Thanks @TkTech for the PR, and @kumaraditya303 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14. 🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington-app[bot] avatar Jul 25 '25 14:07 miss-islington-app[bot]

GH-137105 is a backport of this pull request to the 3.14 branch.

bedevere-app[bot] avatar Jul 25 '25 14:07 bedevere-app[bot]

GH-137106 is a backport of this pull request to the 3.13 branch.

bedevere-app[bot] avatar Jul 25 '25 14:07 bedevere-app[bot]