[subinterpreters] Lingering subinterpreters should be implicitly cleared on shutdown

[ncoghlan]

BPO	36225
Nosy	@ncoghlan, @vstinner, @ned-deily, @encukou, @ericsnowcurrently, @soltysh, @JulienPalard, @nanjekyejoannah, @LewisGaul, @NasaGeek
PRs	gh-80406: Finalise subinterpreters in Py_FinalizeEx() #17575
Files	finalise-subinterps-test.diff

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2019-03-07.13:17:43.665>
labels = ['expert-subinterpreters', '3.10', 'type-crash']
title = '[subinterpreters] Lingering subinterpreters should be implicitly cleared on shutdown'
updated_at = <Date 2021-12-16.23:53:44.369>
user = 'https://github.com/ncoghlan'

bugs.python.org fields:

activity = <Date 2021-12-16.23:53:44.369>
actor = 'nasageek'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Subinterpreters']
creation = <Date 2019-03-07.13:17:43.665>
creator = 'ncoghlan'
dependencies = []
files = ['48733']
hgrepos = []
issue_num = 36225
keywords = ['patch']
message_count = 17.0
messages = ['337392', '337743', '337852', '339149', '339150', '339191', '345802', '349119', '349124', '353873', '357193', '357235', '364587', '371338', '371571', '379366', '379718']
nosy_count = 10.0
nosy_names = ['ncoghlan', 'vstinner', 'ned.deily', 'petr.viktorin', 'eric.snow', 'maciej.szulik', 'mdk', 'nanjekyejoannah', 'LewisGaul', 'nasageek']
pr_nums = ['17575']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue36225'
versions = ['Python 3.10']

[ncoghlan]

https://docs.python.org/3/c-api/init.html#c.Py_EndInterpreter states that "Py_FinalizeEx() will destroy all sub-interpreters that haven’t been explicitly destroyed at that point."

As discussed in hexchat/hexchat#2237, Python 3.7+ doesn't currently do that - it calls Py_FatalError instead.

That change came from #1728, which was based on my initial PEP-432 refactoring work, and I didn't realise that implicitly cleaning up lingering subinterpreters was a documented behaviour.

So I think we should just fix it to behave as documented, and add a new regression test to make sure it doesn't get broken again in the future.

[nanjekyejoannah]

I have been wondering where the regression to test this can be put..in test__xxsubinterpreters.py may be?

[encukou]

Joannah, yes, that looks like a good place. Eric Snow might have more info; he wrote that module.

As for testing Py_FatalError, there's an assert_python_failure function in test.support.script_helper.

[ericsnowcurrently]

Interestingly, I noticed this independently today. :)

Here's what I wrote in bpo-36477 (which I've closed as a duplicate):

When using subinterpreters, any that exist when Py_FinalizeEx() is called do not appear to get cleaned up during runtime finalization. Maybe I've been looking at the code too much and I'm missing something. :)

This really isn't a problem except for embedders that use subinterpreters (where we're leaking memory). However, even with the "python" executable it can have an impact because the subinterpreters' non-daemon threads will exit later than expected. (see bpo-36469 & bpo-36476)

The solution would be to finalize all subinterpreters at the beginning of Py_FinalizeEx(), right before the call to wait_for_thread_shutdown(). This means calling Py_EndInterpreter() for all the runtime's interpreters (except the main one). It would also mean setting a flag (_PyRuntime.interpreters.finalizing?) right before that to disallow creation of any more subinterptreters.

[ericsnowcurrently]

test__xxsubinterpreters is a great place for tests that exercise use of subinterpreters, including most lifecycle operations. There are also one or two subinterpreter-related tests in test_embed. However, for this issue the interplay with runtime finalization means tests should probably stay with other tests that exercise Py_FinalizeEx().

[ncoghlan]

I think test_embed would be the right home for this, as there's an existing test case there for subinterpreter lifecycles and repeated init/finalize cycles:

cpython/Programs/_testembed.c

Line 43 in ddbb978

static int test_repeated_init_and_subinterpreters(void)

The test case here would be similar, but it wouldn't need the outer loop - it would just create a handful of subinterpreters, but instead of ending each one before creating the next one the way the existing test does, what it would instead do is:

• setup as per the existing test case
• create a pair of subinterpeters, using a copy of the existing loop, but omitting the Py_EndInterpreter call
• switch back to the main interpreter
• create a second pair of subinterpeters
• switch back to the main interpreter
• call Py_Finalize

It also occurs to me that we don't currently have a test case for what happens if you call Py_Finalize from a subinterpreter rather than the main interpreter.

[ned-deily]

Ping. It was marked as a 3.7regression but perhaps it should now be just targeted for 3.8.

[nanjekyejoannah]

I am investigating this but in the meantime.

It also occurs to me that we don't currently have a test case for what happens if you call Py_Finalize from a sub-interpreter rather than the main interpreter.

@ncoghlan Am moving this test request in a new issue. So that this issue only focuses on fixing the lingering sub-interpreters.

[nanjekyejoannah]

The test request is moved to bpo-37776.

[nanjekyejoannah]

I remember julien wanting to check this out during a discussion we had at the sprints hence the loop in.

[LewisGaul]

I've put together a test along the lines of what Nick suggested, see the attached patch. Running this hits the Fatal 'remaining subinterpreters' error as expected:

   > ./Programs/_testembed test_bpo36225
--- Pass 0 ---
interp 0 <0x1A561A0>, thread state <0x1A56DD0>: id(modules) = 139707107673104
interp 1 <0x1A9D050>, thread state <0x1A87620>: id(modules) = 139707106987472
interp 2 <0x1B02210>, thread state <0x1AEC320>: id(modules) = 139706981531088
interp 0 <0x1A561A0>, thread state <0x1A56DD0>: id(modules) = 139707107673104
interp 3 <0x1B53740>, thread state <0x1AFD980>: id(modules) = 139706980408304
interp 4 <0x1BA3390>, thread state <0x1B3C7A0>: id(modules) = 139706979780944
interp 0 <0x1A561A0>, thread state <0x1A56DD0>: id(modules) = 139707107673104
Fatal Python error: PyInterpreterState_Delete: remaining subinterpreters
Python runtime state: finalizing (tstate=0x1a56dd0)

Aborted

I'm happy to look a bit further into the fix for this - Eric's pointers in this thread look useful to get started. @nanjekyejoannah did you get anywhere with this?