FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

[3.11] gh-108987: Fix _thread.start_new_thread() race condition (#109135) by vstinner · Pull Request #109272 · python/cpython · GitHub

Repository navigation

[3.11] gh-108987: Fix _thread.start_new_thread() race condition (#109135) - #109272

Merged
vstinner merged 1 commit into
python:3.11from
vstinner:start_new_thread311
Sep 11, 2023
Merged

vstinner merged 1 commit into
python:3.11from
vstinner:start_new_thread311

Conversation

vstinner commented Sep 11, 2023 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Member

Fix _thread.start_new_thread() race condition. If a thread is created during Python finalization, the newly spawned thread now exits immediately instead of trying to access freed memory and lead to a crash.

thread_run() calls PyEval_AcquireThread() which checks if the thread must exit. The problem was that tstate was dereferenced earlier in _PyThreadState_Bind() which leads to a crash most of the time.

Move _PyThreadState_CheckConsistency() from thread_run() to _PyThreadState_Bind().

(cherry picked from commit 517cd82)

…n#109135)

Fix _thread.start_new_thread() race condition. If a thread is created
during Python finalization, the newly spawned thread now exits
immediately instead of trying to access freed memory and lead to a
crash.

thread_run() calls PyEval_AcquireThread() which checks if the thread
must exit. The problem was that tstate was dereferenced earlier in
_PyThreadState_Bind() which leads to a crash most of the time.

Move _PyThreadState_CheckConsistency() from thread_run() to
_PyThreadState_Bind().

(cherry picked from commit 517cd82)

Copy link
Copy Markdown
Member Author

I tested manually this fix, it works as I expected.

I used this test: #109135 (comment)

Without this change: CRASH!

(...)
-- sleep 100 ms before PyEval_AcquireThread() --
-- sleep 100 ms before PyEval_AcquireThread() --
-- sleep 100 ms before PyEval_AcquireThread() --
-- sleep 100 ms before PyEval_AcquireThread() --
-- sleep 100 ms before PyEval_AcquireThread() --
-- sleep 100 ms before PyEval_AcquireThread() --
-- sleep 100 ms before PyEval_AcquireThread() --
exit
-- sleep 1 sec after free_interpreter() --
python: Python/pystate.c:2259: _PyThreadState_CheckConsistency: Assertion `!_PyMem_IsPtrFreed(tstate->interp)' failed.
python: Python/pystate.c:2259: _PyThreadState_CheckConsistency: Assertion `!_PyMem_IsPtrFreed(tstate->interp)' failed.
python: Python/pystate.c:2259: _PyThreadState_CheckConsistency: Assertion `!_PyMem_IsPtrFreed(tstate->interp)' failed.
python: Python/pystate.c:2259: _PyThreadState_CheckConsistency: Assertion `!_PyMem_IsPtrFreed(tstate->interp)' failed.
Abandon (core dumped)

With this change: not crash.

$ ./python bug.py; echo "exitcode=$?"
(...)
-- sleep 100 ms before PyEval_AcquireThread() --
-- sleep 100 ms before PyEval_AcquireThread() --
-- sleep 100 ms before PyEval_AcquireThread() --
-- sleep 100 ms before PyEval_AcquireThread() --
-- sleep 100 ms before PyEval_AcquireThread() --
-- sleep 100 ms before PyEval_AcquireThread() --
-- sleep 100 ms before PyEval_AcquireThread() --
-- sleep 100 ms before PyEval_AcquireThread() --
exit
-- sleep 1 sec after free_interpreter() --
exitcode=0

vstinner merged commit 82a1806 into python:3.11 Sep 11, 2023
vstinner deleted the start_new_thread311 branch September 11, 2023 17:33
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant


Back | FazBrowse Home | New Git URL