classification
Title: Add _at_fork_reinit() method to locks
Type: Stage: resolved
Components: Library (Lib) Versions: Python 3.9
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Batuhan Taskaya, miss-islington, vstinner
Priority: normal Keywords: patch

Created on 2020-03-27 15:51 by vstinner, last changed 2020-04-21 01:07 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 19191 merged vstinner, 2020-03-27 16:24
PR 19193 merged miss-islington, 2020-03-27 16:51
PR 19194 merged miss-islington, 2020-03-27 16:51
PR 19195 merged vstinner, 2020-03-27 16:52
PR 19196 closed vstinner, 2020-03-27 17:06
Messages (8)
msg365157 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-27 15:51
Using a lock after fork() is unsafe and can crash.

Example of a crash in logging after a fork on AIX:
https://bugs.python.org/issue40068#msg365028

This problem is explained in length in bpo-6721: "Locks in the standard library should be sanitized on fork".

The threading module registers an "at fork" callback: Thread._reset_internal_locks() is called to reset self._started (threading.Event) and self._tstate_lock. The current implementation creates new Python lock objects and forgets about the old ones.

I propose to add a new _at_fork_reinit() method to Python lock objects which reinitializes the native lock internally without having to create a new Python object.

Currently, my implementation basically creates a new native lock object and forgets about the old new (don't call PyThread_free_lock()).

Tomorrow, we can imagine a more efficient impementation using platform specific function to handle this case without having to forget about the old lock.
msg365165 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-27 16:29
_at_fork() has a bug: it creates a _DummyThread instead of a _MainThread. Example:
---
import os, _thread, threading, time

def fork_in_thread():
    pid = os.fork()
    if pid:
        # parent
        os._exit(0)

    # child process
    print(f"child process: {threading.current_thread()=}")
    print(f"child process: {threading._main_thread=}")

print(f"parent process: {threading.current_thread()=}")
print(f"parent process: {threading._main_thread=}")

_thread.start_new_thread(fork_in_thread, ())
# block the main thread: fork_in_thread() exit the process
time.sleep(60)
---

Output:
---
parent process: threading.current_thread()=<_MainThread(MainThread, started 139879200077632)>
parent process: threading._main_thread=<_MainThread(MainThread, started 139879200077632)>
child process: threading.current_thread()=<_DummyThread(Dummy-1, started daemon 139878980245248)>
child process: threading._main_thread=<_DummyThread(Dummy-1, started daemon 139878980245248)>
---

My PR 19191 fix the issue:
---
parent process: threading.current_thread()=<_MainThread(MainThread, started 140583665170240)>
parent process: threading._main_thread=<_MainThread(MainThread, started 140583665170240)>
child process: threading.current_thread()=<_MainThread(MainThread, started 140583445075712)>
child process: threading._main_thread=<_MainThread(MainThread, started 140583445075712)>
---
msg365168 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-27 16:50
New changeset d8ff44ce4cd6f3ec0fab5fccda6bf14afcb25c30 by Victor Stinner in branch 'master':
bpo-40089: Fix threading._after_fork() (GH-19191)
https://github.com/python/cpython/commit/d8ff44ce4cd6f3ec0fab5fccda6bf14afcb25c30
msg365171 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-27 17:07
See also bpo-40091 "Crash in logging._after_at_fork_child_reinit_locks()" that I just created.
msg365176 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-27 17:18
See also bpo-40092: "Crash in _PyThreadState_DeleteExcept() at fork in the process child".
msg365945 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-04-07 21:11
New changeset 87255be6964979b5abdc4b9dcf81cdcfdad6e753 by Victor Stinner in branch 'master':
bpo-40089: Add _at_fork_reinit() method to locks (GH-19195)
https://github.com/python/cpython/commit/87255be6964979b5abdc4b9dcf81cdcfdad6e753
msg365947 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-04-07 21:35
New changeset 6318e45bda6c37d5497f33a6039cdb65aa494c93 by Miss Islington (bot) in branch '3.8':
bpo-40089: Fix threading._after_fork() (GH-19191) (GH-19194)
https://github.com/python/cpython/commit/6318e45bda6c37d5497f33a6039cdb65aa494c93
msg365948 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-04-07 21:36
New changeset a514ccb3ea6f01fef850d9465b82a1670d5ace44 by Miss Islington (bot) in branch '3.7':
bpo-40089: Fix threading._after_fork() (GH-19191) (GH-19193)
https://github.com/python/cpython/commit/a514ccb3ea6f01fef850d9465b82a1670d5ace44
History
Date User Action Args
2020-04-21 01:07:34vstinnersetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2020-04-07 21:36:10vstinnersetmessages: + msg365948
2020-04-07 21:35:55vstinnersetmessages: + msg365947
2020-04-07 21:11:56vstinnersetmessages: + msg365945
2020-03-27 17:18:23vstinnersetmessages: + msg365176
2020-03-27 17:07:09vstinnersetmessages: + msg365171
2020-03-27 17:06:31vstinnersetpull_requests: + pull_request18557
2020-03-27 16:52:14vstinnersetpull_requests: + pull_request18555
2020-03-27 16:51:18miss-islingtonsetpull_requests: + pull_request18554
2020-03-27 16:51:10miss-islingtonsetnosy: + miss-islington
pull_requests: + pull_request18553
2020-03-27 16:50:45vstinnersetmessages: + msg365168
2020-03-27 16:29:48vstinnersetmessages: + msg365165
2020-03-27 16:24:45vstinnersetkeywords: + patch
stage: patch review
pull_requests: + pull_request18551
2020-03-27 15:57:32Batuhan Taskayasetnosy: + Batuhan Taskaya
2020-03-27 15:51:37vstinnercreate