bpo-46210: Fix deadlock in print. by notarealdeveloper · Pull Request #30310 · python/cpython

Hi there. :)

After spending some time thinking about how to exercise the lock re-initialization function in a test, I realized that the best way to overcome my noobish-ness here is to follow the example of _PyImport_ReInitLock, seeing as @vstinner has done a lot of the work on that, and his past fixes of various deadlocks are basically identical to what I'm trying to do here.

The "meat" of the implementation we've already discussed doesn't change (thanks again for the tips @sweeneyde). I've just moved things to different files, in order to maximize the similarity with what _PyImport_ReInitLock does.

Here's what a grep of the codebase for the two functions looks like:

$ grep -Ir _PyImport_ReInitLock
Modules/posixmodule.c:#include "pycore_import.h"        // _PyImport_ReInitLock()
Modules/posixmodule.c:    status = _PyImport_ReInitLock();
Include/internal/pycore_import.h:extern PyStatus _PyImport_ReInitLock(void);
Python/import.c:_PyImport_ReInitLock(void)

$ grep -Ir _PySys_ReInitStdio
Modules/posixmodule.c:#include "pycore_sysmodule.h"     // _PySys_ReInitStdio()
Modules/posixmodule.c:    status = _PySys_ReInitStdio();
Include/internal/pycore_sysmodule.h:extern PyStatus _PySys_ReInitStdio(void);
Python/sysmodule.c:_PySys_ReInitStdio(void)

And here's the git blame output showing the only location where the new function is called.

26881c8fae3 (Victor Stinner  2020-06-02 15:51:37  592)     status = _PyImport_ReInitLock();
26881c8fae3 (Victor Stinner  2020-06-02 15:51:37  593)     if (_PyStatus_EXCEPTION(status)) {
26881c8fae3 (Victor Stinner  2020-06-02 15:51:37  594)         goto fatal_error;
26881c8fae3 (Victor Stinner  2020-06-02 15:51:37  595)     }
26881c8fae3 (Victor Stinner  2020-06-02 15:51:37  596) 
8c8d9c99971 (Jason Wilkes    2022-01-05 11:45:01  597)     status = _PySys_ReInitStdio();
8c8d9c99971 (Jason Wilkes    2022-01-05 11:45:01  598)     if (_PyStatus_EXCEPTION(status)) {
8c8d9c99971 (Jason Wilkes    2022-01-05 11:45:01  599)         goto fatal_error;
8c8d9c99971 (Jason Wilkes    2022-01-05 11:45:01  600)     }

The goal was to get line-by-line correspondence with what @vstinner does with the import lock, and we seem to have that now.

Now, the only difference arises inside the implementation, because the import lock is a global variable inside Python/import.c, while the stdout lock lives on the stdout.buffer object (similarly with stderr). The addition of an _at_fork_reinit method to the buffered object in Modules/_io/bufferedio.c isn't a necessary part of the implementation, it's just the simplest way I knew of (as a noob) to get a pointer to those locks from outside that module. I'd be more than happy to change that detail if anyone thinks there's a better approach.

As for everything else: (1) the PEP 7 formatting should be good now, (2) @the-knights-who-say-ni seem to have given me the "CLA signed" badge just now, (3) in playing with all this, I stumbled on another small unrelated bug in the test suite and fixed it, so I'll submit that as a separate bpo & pull request.

Let me know if there's anything else you'd like me to do. Thanks again for all the help. :)