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

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

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 11, 2023
Merged

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

merged 1 commit into from
Sep 11, 2023

Conversation

@vstinner
Copy link
Member

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

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)

@vstinner
pythongh-108987: Fix _thread.start_new_thread() race condition (pytho…
184cf7c 
…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)
This was referenced Sep 11, 2023
Closed
Merged
@vstinner
Copy link
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 vstinner merged commit 82a1806 into python:3.11 Sep 11, 2023
@vstinner vstinner deleted the start_new_thread311 branch September 11, 2023 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@vstinner