8000 gh-80406: Finalise subinterpreters in Py_FinalizeEx() by LewisGaul · Pull Request #17575 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

gh-80406: Finalise subinterpreters in Py_FinalizeEx() #17575

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

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open

gh-80406: Finalise subinterpreters in Py_FinalizeEx() #17575

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
23af5f5
Add test suggested by ncoghlan
LewisGaul Nov 21, 2019
433663c
Finalise sub-interpreters in Py_FinalizeEx()
LewisGaul Dec 11, 2019
48e1cfc
Improve test name
LewisGaul Dec 13, 2019
0400634
Switch back to main threadstate in test_audit_subinterpreter before c…
LewisGaul Dec 13, 2019
b79649c
📜🤖 Added by blurb_it.
blurb-it[bot] Dec 14, 2019
8b1e7d9
Markups including: switch from 'finalizing' flag to 'allow_new', add …
LewisGaul Jan 21, 2020
fd6073a
Merge branch 'finalise-subinterps' of github.com:LewisGaul/cpython in…
LewisGaul Jan 21, 2020
4bbd58f
Merge branch 'master' into finalise-subinterps
LewisGaul Oct 20, 2020
1095e66
Use '_' for unused variable in test_embed.py
LewisGaul Oct 20, 2020
675285d
Fix struct position of 'allow_new' flag
LewisGaul Oct 22, 2020
8e21788
Add handling for unsupported case of calling Py_Finalize() from a sub…
LewisGaul Oct 22, 2020
606c068
Emit resource warning when calling Py_Finalize() with unfinalized sub…
LewisGaul Oct 22, 2020
e0789b0
Update Py_FinalizeEx() docs
LewisGaul Oct 22, 2020
dda99ce
Update test for resource warning when implicitly finalizing subinterp…
LewisGaul Oct 23, 2020
847e8d2
Tidy up test_finalize_subinterps() testcase
LewisGaul Oct 23, 2020
a2fb0fc
Add testcase for calling Py_Finalize() from a subinterpreter
LewisGaul Oct 23, 2020
d234528
Tweak subinterpreters still running ResourceWarning handling
LewisGaul Nov 23, 2020
46a8619
Make calling PyFinalizeEx() from a subinterpreter a Py_FatalError
LewisGaul Nov 23, 2020
c89c0e5
Acquire interpreters mutex before setting allow_new=0 in PyFinalizeEx()
LewisGaul Nov 23, 2020
c285f52
Merge remote-tracking branch 'upstream/master' into finalise-subinterps
LewisGaul Nov 23, 2020
95cbfd4
Add back in the 'interp' variable to PyFinalizeEx() to fix the build
LewisGaul Nov 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Finalise sub-interpreters in Py_FinalizeEx()
  • Loading branch information
@LewisGaul
LewisGaul committed Dec 11, 2019
commit 433663cbac435f9fc05b44cb25bd5843f1396354
1 change: 1 addition & 0 deletions Include/internal/pycore_pystate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ typedef struct pyruntimestate {
If that becomes a problem later then we can adjust, e.g. by
using a Python int. */
int64_t next_id;
int finalizing;
} interpreters;
// XXX Remove this field once we have a tp_* slot.
struct _xidregistry {
Expand Down
18 changes: 18 additions & 0 deletions Python/pylifecycle.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1257,6 +1257,20 @@ Py_FinalizeEx(void)
PyThreadState *tstate = _PyRuntimeState_GetThreadState(runtime);
PyInterpreterState *interp = tstate->interp;

// Finalize sub-interpreters.
runtime->interpreters.finalizing = 1;
PyInterpreterState *subinterp = PyInterpreterState_Head();
PyInterpreterState *next_interp;
while (subinterp != NULL) {
next_interp = PyInterpreterState_Next(subinterp);
if (subinterp != PyInterpreterState_Main()) {
PyThreadState_Swap(subinterp->tstate_head);
Py_EndInterpreter(subinterp->tstate_head);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fails if the interp.tstate_head is still running (has a frame). We may want to consider a more graceful approach to dealing with subinterpreters that are still doing work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm yes, do you have any further thoughts on this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll have to give it more thought (and take the time to queue up details into my mental cache 😄).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've recently hit this problem with lingering subinterpreter with existing frames.

I solved it by adding Py_CLEAR before the Py_EndInterpreter call (which is similar to how PyThreadState_Clear handles existing frames) and now it seems to work and end gracefully. While that might not be the best solution (and I don't see in subinterpreters much to see possible issues with that), I guess it's not worse than Fatal error.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ericsnowcurrently any new thoughts here? We might be better off having a call to chat through everything :)

}
subinterp = next_interp;
}
PyThreadState_Swap(tstate);

// Wrap up existing "threading"-module-created, non-daemon threads.
wait_for_thread_shutdown(tstate);

Expand Down Expand Up @@ -1454,6 +1468,10 @@ new_interpreter(PyThreadState **tstate_p)
return _PyStatus_ERR("Py_Initialize must be called first");
}

if (runtime->interpreters.finalizing) {
return _PyStatus_ERR("Interpreters are being finalized");
}

/* Issue #10915, #15751: The GIL API doesn't work with multiple
interpreters: disable PyGILState_Check(). */
_PyGILState_check_enabled = 0;
Expand Down
0