[subinterpreters] PEP 554 implementation: add interpreters module

[ericsnowcurrently]

BPO	32604
Nosy	@ncoghlan, @abalkin, @pitrou, @vstinner, @pmp-p, @ericsnowcurrently, @zware, @zooba, @applio, @emilyemorehouse, @pablogsal, @miss-islington, @nanjekyejoannah
PRs	bpo-32604: Expose the subinterpreters C-API in a "private" stdlib module. #1748 bpo-32604: Add the _xxsubinterpreters extension module under Windows. #5436 bpo-32604: NULL-terminate kwlist in channel_drop_interpreter(). #5437 bpo-32604: Fix memory leaks in the new _xxsubinterpreters module. #5507 [3.7] bpo-32604: Fix memory leaks in the new _xxsubinterpreters module. (GH-5507) #5509 bpo-32604: Make _xxsubinterpreters build on Windows #5516 [3.7] bpo-32604: Make _xxsubinterpreters build on Windows (GH-5516) #5624 bpo-32604: Clean up created subinterpreters before runtime finalization. #5709 [3.7] bpo-32604: Clean up created subinterpreters before runtime finalization. (gh-5709) #5710 bpo-32604: Swap threads only if the interpreter is different. #5778 [3.7] bpo-32604: Swap threads only if the interpreter is different. (gh-5778) #5783 bpo-32604: Remove xid registry. #6813 bpo-32604: Improve subinterpreter tests. #6914 bpo-32604: Implement force-closing channels. #6937 bpo-33615: avoid extra decref #7251 bpo-33615: Temporarily disable a test that is triggering crashes on a few buildbots. #7288 bpo-33724: Use the right format code for int64_t in subinterpreters code. #7330 bpo-32604: [_xxsubinterpreters] Propagate exceptions. #19768 bpo-32604: Add support for a "default" arg in channel_recv(). #19770 bpo-32604: [_xxsubinterpreters] Add channel_send_wait(). #19829 Revert "bpo-32604: [_xxsubinterpreters] Propagate exceptions. (GH-19768)" #20089 bpo-32604: Multiple Interpreters in the Stdlib (PEP 554) #18817 bpo-32604: PEP 554 for use in test suite #19985 Revert "bpo-32604: PEP 554 for use in test suite (GH-19985)" #20465 bpo-32604: Fix reference leak in select module #20600 bpo-32604: Recommit "bpo-32604: PEP 554 for use in test suite (GH-19985)" #20611 bpo-32604: Disable failing tests #20777 bpo-32604: Clean up test.support.interpreters. #20926

^{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 = 'https://github.com/ericsnowcurrently'
closed_at = None
created_at = <Date 2018-01-20.01:58:01.942>
labels = ['expert-subinterpreters', 'type-feature', '3.10', 'docs']
title = '[subinterpreters] PEP 554 implementation: add interpreters module'
updated_at = <Date 2020-06-17.00:42:48.323>
user = 'https://github.com/ericsnowcurrently'

bugs.python.org fields:

activity = <Date 2020-06-17.00:42:48.323>
actor = 'eric.snow'
assignee = 'eric.snow'
closed = False
closed_date = None
closer = None
components = ['Documentation', 'Subinterpreters']
creation = <Date 2018-01-20.01:58:01.942>
creator = 'eric.snow'
dependencies = []
files = []
hgrepos = []
issue_num = 32604
keywords = ['patch']
message_count = 66.0
messages = ['310314', '310316', '310317', '310318', '310448', '310562', '311210', '311211', '311212', '311217', '311218', '311219', '311220', '311222', '311224', '311299', '311533', '311534', '311536', '311538', '312001', '312004', '312260', '312262', '312288', '312362', '312367', '312368', '312369', '312370', '312372', '312443', '312447', '312837', '312888', '312975', '312976', '312979', '313010', '316657', '316717', '316850', '316936', '318333', '318479', '319560', '367573', '368349', '368395', '368846', '368849', '368851', '368899', '368902', '369378', '369454', '369455', '370130', '370140', '370631', '370660', '371157', '371173', '371176', '371704', '371707']
nosy_count = 13.0
nosy_names = ['ncoghlan', 'belopolsky', 'pitrou', 'vstinner', 'pmpp', 'eric.snow', 'zach.ware', 'steve.dower', 'davin', 'emilyemorehouse', 'pablogsal', 'miss-islington', 'nanjekyejoannah']
pr_nums = ['1748', '5436', '5437', '5507', '5509', '5516', '5624', '5709', '5710', '5778', '5783', '6813', '6914', '6937', '7251', '7288', '7330', '19768', '19770', '19829', '20089', '18817', '19985', '20465', '20600', '20611', '20777', '20926']
priority = None
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue32604'
versions = ['Python 3.10']

[ericsnowcurrently]

In the interest of getting something landed for 3.7, so we can start using it in tests, I'm putting up a patch for a low-level interpreters module. In some ways this is a precursor for issue bpo-30439, which will add a proper public stdlib module in 3.8.

The module I'm adding draws from the ideas in PEP-554 (particularly for channels). Consequently, this will also give us an opportunity to try out some of the semantics from the PEP to give us better ideas for 3.8.

I expect to have some follow-on patches to facilitate simpler use in tests. This patch is big enough already. :)

[ericsnowcurrently]

@ned, it may be a little tight to land this given the time left before beta 1. However, this is meant as a tool for us to use in the test suite (particularly to test the subinterpreter C-API). So I'm arguing that, if necessary, it would still be okay to land this after the feature freeze. (I'm still hoping to get this in before the cutoff.) What do you think?

[ericsnowcurrently]

@nick, I may make the name change you suggested in issue bpo-30439 ("_subinterpreters").

[ericsnowcurrently]

FYI, there are a few things I need to clean up in the PR. However, I expect that those changes will be minor relative to the the whole patch, so I wanted to get the ball rolling on a review. :)

[ned-deily]

@eric, given the breadth of change introduced in the PR (including adding a new extension), I think it would be best if at all possible to get it in for beta 1 if we can resolve the review comments in time. If necessary and if there are no objections from other core developers, I would be willing to consider making an exception and allowing it into beta 2 as long as it remains a private interface. If it looks like it won't be in releasable shape by then, I think you should hold off for 3.8; doing otherwise would be unfair to others and to our downstream beta users / testers, for example, even if it is private, adding a new extension and setup.py changes potentially affect downstream packagers.

[ericsnowcurrently]

Sounds good, Ned. Thanks for taking a look. I should have everything finished up by Friday, so I'm hopeful for landing the change before the deadline. I may have a few minor tweaks to make after that, but I'll discuss that with you before making any changes if that happens.

[ericsnowcurrently]

New changeset 7f8bfc9 by Eric Snow in branch 'master':
bpo-32604: Expose the subinterpreters C-API in a "private" stdlib module. (gh-1748)
7f8bfc9

[ericsnowcurrently]

I've merged the patch without Windows support, which shouldn't be a problem given the purpose of the extension module. I've also added a PR for get the module building under Windows. I'd like to get that resolved ASAP.

[ned-deily]

Eric, looks like some buildbots are unhappy, for instance:

http://buildbot.python.org/all/#builders/13/builds/648

[ericsnowcurrently]

Yeah, I'm looking into it. Also, I noticed some refleaks that I'll be sorting out.

[ericsnowcurrently]

On 4 of the buildbots:

======================================================================
ERROR: test_drop_multiple_times (test.test__xxsubinterpreters.ChannelTests)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "/srv/buildbot/buildarea/3.x.bolen-ubuntu/build/Lib/test/test__xxsubinterpreters.py", line 890, in test_drop_multiple_times
    interpreters.channel_drop_interpreter(cid, send=True, recv=True)
SystemError: More keyword list entries (7) than format specifiers (3)

======================================================================
ERROR: test_drop_single_user (test.test__xxsubinterpreters.ChannelTests)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "/srv/buildbot/buildarea/3.x.bolen-ubuntu/build/Lib/test/test__xxsubinterpreters.py", line 848, in test_drop_single_user
    interpreters.channel_drop_interpreter(cid, send=True, recv=True)
SystemError: More keyword list entries (7) than format specifiers (3)

======================================================================
ERROR: test_drop_used_multiple_times_by_single_user (test.test__xxsubinterpreters.ChannelTests)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "/srv/buildbot/buildarea/3.x.bolen-ubuntu/build/Lib/test/test__xxsubinterpreters.py", line 957, in test_drop_used_multiple_times_by_single_user
    interpreters.channel_drop_interpreter(cid, send=True, recv=True)
SystemError: More keyword list entries (7) than format specifiers (3)

======================================================================
ERROR: test_drop_with_unused_items (test.test__xxsubinterpreters.ChannelTests)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "/srv/buildbot/buildarea/3.x.bolen-ubuntu/build/Lib/test/test__xxsubinterpreters.py", line 899, in test_drop_with_unused_items
    interpreters.channel_drop_interpreter(cid, send=True, recv=True)
SystemError: More keyword list entries (7) than format specifiers (3)

[ericsnowcurrently]

On the PPC64 AIX 3.x buildbot:

======================================================================
FAIL: test_repr (test.test__xxsubinterpreters.ChannelIDTests)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "/home/shager/cpython-buildarea/3.x.edelsohn-aix-ppc64/build/Lib/test/test__xxsubinterpreters.py", line 784, in test_repr
    self.assertEqual(repr(cid), 'ChannelID(10)')
AssertionError: 'ChannelID(0)' != 'ChannelID(10)'
- ChannelID(0)
+ ChannelID(10)
?           +

[ericsnowcurrently]

I just put up a PR that should fix the 4 buildbots.

[ericsnowcurrently]

New changeset 83e64c8 by Eric Snow in branch 'master':
bpo-32604: NULL-terminate kwlist in channel_drop_interpreter(). (gh-5437)
83e64c8

[ericsnowcurrently]

The buildbots should be happier now. I'll keep an eye on them.

[1st1]

A couple defects reported by coverity:

** CID 1428758: Integer handling issues (CONSTANT_EXPRESSION_RESULT) /Modules/_xxsubinterpretersmodule.c: 45 in _coerce_id()
________________________________________________________________________________________________________ *** CID 1428758: Integer handling issues (CONSTANT_EXPRESSION_RESULT) /Modules/_xxsubinterpretersmodule.c: 45 in _coerce_id()
39 }
40 if (cid < 0) {
41 PyErr_SetString(PyExc_ValueError, 42 "'id' must be a non-negative int"); 43 return -1;
44 }
45 if (cid > INT64_MAX) {
46 PyErr_SetString(PyExc_ValueError, 47 "'id' too large (must be 64-bit int)"); 48 return -1;
49 }
50 return cid;
** CID 1428757: Integer handling issues (CONSTANT_EXPRESSION_RESULT) /Modules/_xxsubinterpretersmodule.c: 1215 in channelid_richcompare()
________________________________________________________________________________________________________ *** CID 1428757: Integer handling issues (CONSTANT_EXPRESSION_RESULT) /Modules/_xxsubinterpretersmodule.c: 1215 in channelid_richcompare()
1209 }
1210 int64_t othercid = PyLong_AsLongLong(other); 1211 // XXX decref other here?
1212 if (othercid == -1 && PyErr_Occurred() != NULL) { 1213 return NULL;
1214 }
1215 if (othercid < 0 || othercid > INT64_MAX) { 1216 equal = 0;

1217 }
1218 else {
1219 equal = (cid->id == othercid); 1220 }
** CID 1428756: Error handling issues (CHECKED_RETURN) /Modules/gcmodule.c: 1071 in gc_enable_impl()

[ericsnowcurrently]

New changeset 4e9da0d by Eric Snow in branch 'master':
bpo-32604: Fix memory leaks in the new _xxsubinterpreters module. (bpo-5507)
4e9da0d

[ericsnowcurrently]

I've landed a PR that fixes all the memory leaks in the module. It also fixes the 2 defects reported by coverity. The only thing left here is to get the module building under Windows.

[ericsnowcurrently]

New changeset f33eced by Eric Snow (Miss Islington (bot)) in branch '3.7':
bpo-32604: Fix memory leaks in the new _xxsubinterpreters module. (GH-5507)
f33eced

[ericsnowcurrently]

FYI, out of 2389 source lines in the C extension, 1563 are the channel-related code. That means the non-channel code is 826 lines (about a third). That non-channel code does not depend on the channel code at all and I considered splitting the source out, but figured there wasn't enough benefit. However, I might revisit the matter later when I circle back to PEP-554. :)

[zware]

New changeset 310b052 by Zachary Ware in branch 'master':
bpo-32604: Make _xxsubinterpreters build on Windows (GH-5516)
310b052

[zware]

New changeset fe61e8d by Zachary Ware (Miss Islington (bot)) in branch '3.7':
bpo-32604: Make _xxsubinterpreters build on Windows (GH-5624)
fe61e8d

[ericsnowcurrently]

New changeset 4c6955e by Eric Snow in branch 'master':
bpo-32604: Clean up created subinterpreters before runtime finalization. (gh-5709)
4c6955e

[ericsnowcurrently]

New changeset 3db05a3 by Eric Snow (Miss Islington (bot)) in branch '3.7':
bpo-32604: Clean up created subinterpreters before runtime finalization. (gh-5710)
3db05a3

[1st1]

Eric, it looks like your recent commit introduced a refleak. We need to fix it before beta2.

~/d/p/cpython (master $) » ./python.exe -m test -R3:3 test_multiprocessing_fork
Run tests sequentially
0:00:00 load avg: 2.52 [1/1] test_multiprocessing_fork
beginning 6 repetitions
123456
......
test_multiprocessing_fork leaked [21, 2, 1] memory blocks, sum=24
test_multiprocessing_fork leaked [2, 0, 0] file descriptors, sum=2
test_multiprocessing_fork failed in 9 min 48 sec

1 test failed:
test_multiprocessing_fork

And just before it:

~/d/p/cpython ((bd09335…) $) » ./python.exe -m test -R3:3 test_multiprocessing_fork
Run tests sequentially
0:00:00 load avg: 3.70 [1/1] test_multiprocessing_fork
beginning 6 repetitions
123456
......
test_multiprocessing_fork passed in 9 min 12 sec
1 test OK.

Total duration: 9 min 12 sec
Tests result: SUCCESS

[ericsnowcurrently]

Also see bpo-24553.

[miss-islington]

New changeset 5e8c691 by Eric Snow in branch 'master':
bpo-32604: Add support for a "default" arg in channel_recv(). (GH-19770)
5e8c691

[ericsnowcurrently]

New changeset a1d9e0a by Eric Snow in branch 'master':
bpo-32604: [_xxsubinterpreters] Propagate exceptions. (GH-19768)
a1d9e0a

[vstinner]

The commit a1d9e0a introduced reference leaks:

$ ./python -m test -R 3:3 test__xxsubinterpreters -m test.test__xxsubinterpreters.RunFailedTests.test_traceback_propagated
(...)
test__xxsubinterpreters leaked [1863, 1861, 1863] references, sum=5587
test__xxsubinterpreters leaked [559, 558, 559] memory blocks, sum=1676
(...)

[vstinner]

The buildbots are broken for one whole week and nobody is available to investigate the reference leak regression. Following the CI policy, I wrote PR 20089 to revert the change which broke the CI:
https://pythondev.readthedocs.io/ci.html#revert-on-fail

If someone wants to push again the change, make sure that "./python -m test -R 3:3 test__xxsubinterpreters" does not leak.

[vstinner]

New changeset f2c3b68 by Victor Stinner in branch 'master':
Revert "bpo-32604: [_xxsubinterpreters] Propagate exceptions. (GH-19768)" (GH-20089)
f2c3b68

[vstinner]

The revert fixed the test:

$ ./python -m test -R 3:3 test__xxsubinterpreters 
(...)
Tests result: SUCCESS

[vstinner]

I marked bpo-30439 and bpo-31375 as duplicates of this issue.

[vstinner]

I marked bpo-39881 as a duplicate of this issue. It has two open PRs:

• PR 18817
• PR 19985

I changed their title to use bpo-32604.

[nanjekyejoannah]

New changeset 9d17cbf by Joannah Nanjekye in branch 'master':
bpo-32604: PEP-554 for use in test suite (GH-19985)
9d17cbf

[vstinner]

The new test leaks references:

https://buildbot.python.org/all/#/builders/563/builds/105

test_interpreters leaked [216, 216, 216] references, sum=648
test_interpreters leaked [84, 84, 84] memory blocks, sum=252

Use "./python -m test.bisect_cmd -R 3:3 test_interpreters" to find tests which leak. Example:

$ ./python -m test test_interpreters -R 3:3 -m test.test_interpreters.TestInterpreterDestroy.test_from_current
0:00:00 load avg: 0.82 Run tests sequentially
0:00:00 load avg: 0.82 [1/1] test_interpreters
beginning 6 repetitions
123456
......
test_interpreters leaked [36, 36, 36] references, sum=108
test_interpreters leaked [14, 14, 14] memory blocks, sum=42
test_interpreters failed

== Tests result: FAILURE ==

1 test failed:
test_interpreters

Total duration: 1.3 sec
Tests result: FAILURE

[nanjekyejoannah]

I will look at this tommorrow. Am abit busy today. Thanks Victor

[pablogsal]

Commit 9d17cbf is causing all refleak buildbots to fail and is masking other issues and causing some confusion already in other PRs (like #20433) so I opened a revert in PR 20465 following the buildbot revert policy (https://discuss.python.org/t/policy-to-revert-commits-on-buildbot-failure/404).

Please, open a new PR with this change again once the refleaks have been resolved.

[vstinner]

New changeset 7d80b35 by Pablo Galindo in branch 'master':
Revert "bpo-32604: PEP-554 for use in test suite (GH-19985)" (bpo-20465)
7d80b35

[vstinner]

The following test is enough to reproduce the leak:
---

import unittest
import _xxsubinterpreters as _interpreters

class TestInterpreterDestroy(unittest.TestCase):

    def tearDown(self):
        for interp_id in _interpreters.list_all():
            if interp_id == 0:  # main
                continue
            _interpreters.destroy(interp_id)

    def test_from_current(self):
        interp_id = _interpreters.create(isolated=True)
        _interpreters.run_string(interp_id, "import select")

---

[vstinner]

New changeset 18a9024 by Victor Stinner in branch 'master':
bpo-32604: Fix reference leak in select module (GH-20600)
18a9024

[nanjekyejoannah]

New changeset bae872f by Joannah Nanjekye in branch 'master':
bpo-32604: Recommit "bpo-32604: PEP-554 for use in test suite (GH-19985)" (GH-20611)
bae872f

[vstinner]

test__xxsubinterpreters and test_interpreters failed on AMD64 FreeBSD Shared 3.x:
https://buildbot.python.org/all/#/builders/152/builds/973

See also bpo-37224: "[subinterpreters] test__xxsubinterpreters fails randomly".

IMO there are multiple race conditions in these tests.

======================================================================
FAIL: test_subinterpreter (test.test__xxsubinterpreters.IsRunningTests)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "/usr/home/buildbot/python/3.x.koobs-freebsd-564d/build/Lib/test/test__xxsubinterpreters.py", line 478, in test_subinterpreter
    self.assertTrue(interpreters.is_running(interp))
AssertionError: False is not true

======================================================================
FAIL: test_still_running (test.test_interpreters.TestInterpreterDestroy)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "/usr/home/buildbot/python/3.x.koobs-freebsd-564d/build/Lib/test/test_interpreters.py", line 309, in test_still_running
    interp.close()
AssertionError: RuntimeError not raised

======================================================================
FAIL: test_subinterpreter (test.test_interpreters.TestInterpreterIsRunning)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "/usr/home/buildbot/python/3.x.koobs-freebsd-564d/build/Lib/test/test_interpreters.py", line 207, in test_subinterpreter
    self.assertTrue(interp.is_running())
AssertionError: False is not true

======================================================================
FAIL: test_already_running (test.test_interpreters.TestInterpreterRun)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "/usr/home/buildbot/python/3.x.koobs-freebsd-564d/build/Lib/test/test_interpreters.py", line 383, in test_already_running
    self.interp.run('print("spam")')
AssertionError: RuntimeError not raised

[pablogsal]

Joannah, Eric, could you look into this? Having random failures in the builbots can make debugging more challenging, hide other issues and can also spam the buildbot list :(

[ericsnowcurrently]

New changeset 818f5b5 by Eric Snow in branch 'master':
bpo-32604: Clean up test.support.interpreters. (gh-20926)
818f5b5

[ericsnowcurrently]

@pablo, yeah, I'll try to take a look this week. Are these new failures?

Keep in mind that these tests can sometimes expose existing bugs in our runtime (as we fix runtime issues elsewhere) rather than regressions, though that isn't necessarily the case here. :)