gh-113964: Don't prevent new threads until all non-daemon threads exit

[colesbury]

Starting in Python 3.12, we started preventing fork() and starting new threads during interpreter finalization (shutdown). This has led to a number of regressions and flaky tests. We should not prevent starting new threads (or fork()) until all non-daemon threads exit and finalization starts in earnest.

This changes the checks to use
_PyInterpreterState_GetFinalizing(interp), which is set immediately before terminating non-daemon threads.

• Issue: 3.12 regression: "can't create new thread at interpreter shutdown" from non-daemon threads or atexit handlers #113964

[colesbury]

This also fixes the flaky test test_concurrent_futures.test_shutdown (#112542).

See also the discussion #114570

[gvanrossum]

Thanks for looking at this! I'll leave it up to @gpshead to review.

[vstinner]

History of _thread changes.

(1) June 2023, issue gh-104690: _thread.start_new_thread() disallows the creation of new thread during Python finalization since the commit ce558e6 to fix issue gh-104690 which caused a crash.

(2) Sept 2023, issue gh-108987: thread_run() of Modules/_threadmodule.c now starts with:

    // gh-108987: If _thread.start_new_thread() is called before or while
    // Python is being finalized, thread_run() can called *after*.
    // _PyRuntimeState_SetFinalizing() is called. At this point, all Python
    // threads must exit, except of the thread calling Py_Finalize() whch holds
    // the GIL and must not exit.
    //
    // At this stage, tstate can be a dangling pointer (point to freed memory),
    // it's ok to call _PyThreadState_MustExit() with a dangling pointer.
    if (_PyThreadState_MustExit(tstate)) {
        // Don't call PyThreadState_Clear() nor _PyThreadState_DeleteCurrent().
        // These functions are called on tstate indirectly by Py_Finalize()
        // which calls _PyInterpreterState_Clear().
        //
        // Py_DECREF() cannot be called because the GIL is not held: leak
        // references on purpose. Python is being finalized anyway.
        thread_bootstate_free(boot, 0);
        goto exit;
    }

(3) Feb 2024, gh-114570: I added PythonFinalizationError exception in commit 3e7b7df.

With all these changes, I'm no longer sure what should be the correct behavior. I only hope that our test suite is now testing cases which caused crashes previously.

I prefer to let someone else decide on this issue :-)

The Python finalization is fragile. I took notes on old issues and the history of changes at: https://pythondev.readthedocs.io/finalization.html

[gpshead]

In general I think this change is correct. I believe we'll want to backport this to 3.12 (I haven't looked at how difficult that'll be though) as the issue came up there as being a behavior regression from <=3.11 that reasonable code relies on.

One thing I think we're missing with the unit test transformations to be "AtFinalization" via the main interpreter exiting and triggering is: Alternate explicit tests for atexit.register()'d handlers at finalization time. Which implies we need to explicitly define what that behavior should be. But that may be a separate PR, i've noted that on the issue.

For this, see also #115219 which I may mark a dupe.

[gpshead]

I created #116982 for atexit - please take a look at that. Keeping these separate makes sense, and yours should go in first. But it'll be an easier merge if the tests I build on are not removed if feasible. (otherwise no worries, i'll untangle it when merging into mine)

[colesbury]

@gpshead - I don't see a way to avoid removing/replacing the atexit tests. The existing atexit tests would not pass with this PR (because the exception is not raised), and the atexit tests from your PR would not pass reliably without the other changes from your PR.

[colesbury]

I will go ahead and merge this with apologies for the inevitable merge conflicts.

[miss-islington-app]

Thanks @colesbury for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

[miss-islington-app]

Sorry, @colesbury, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 60e105c1c11ecca1680d03c38aa06bcc77a28714 3.12

[bedevere-app]

GH-117029 is a backport of this pull request to the 3.12 branch.