cpython
cpython copied to clipboard
python
[subinterpreters] PEP 554 implementation: add interpreters module
| BPO | 32604 |
|---|---|
| Nosy | @ncoghlan, @abalkin, @pitrou, @vstinner, @pmp-p, @ericsnowcurrently, @zware, @zooba, @applio, @emilyemorehouse, @pablogsal, @miss-islington, @nanjekyejoannah |
| PRs |
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']
Linked PRs
- gh-109556
- gh-109586
- gh-110236
- gh-110246
- gh-110248
- gh-110251
- gh-110314
- gh-110322
- gh-110568
- gh-110606
- gh-110607
- gh-111502
- gh-111530
- gh-111572
- gh-111573
- gh-111575
- gh-111671
- gh-111715
- gh-112288
- gh-112322
- gh-112323
- gh-112982
- gh-113012
- gh-113014
- gh-113021
- gh-113034
- gh-113036
- gh-113038
- gh-113470
- gh-113473
- gh-115424
- gh-115566
- gh-116166
- gh-116318
- gh-116328
- gh-116369
- gh-116430
- gh-116431
- gh-117048
- gh-117101
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. :)
@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?
@Nick, I may make the name change you suggested in issue bpo-30439 ("_subinterpreters").
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. :)
@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.
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.
New changeset 7f8bfc9b9a8381ddb768421b5dd5cbd970266190 by Eric Snow in branch 'master': bpo-32604: Expose the subinterpreters C-API in a "private" stdlib module. (gh-1748) https://github.com/python/cpython/commit/7f8bfc9b9a8381ddb768421b5dd5cbd970266190
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.
Eric, looks like some buildbots are unhappy, for instance:
http://buildbot.python.org/all/#builders/13/builds/648
Yeah, I'm looking into it. Also, I noticed some refleaks that I'll be sorting out.
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)
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)
? +
New changeset 83e64c8a544028ae677af2a0bc268dbe1c11cc3a by Eric Snow in branch 'master': bpo-32604: NULL-terminate kwlist in channel_drop_interpreter(). (gh-5437) https://github.com/python/cpython/commit/83e64c8a544028ae677af2a0bc268dbe1c11cc3a
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()
New changeset 4e9da0d163731caa79811c723c703ee416c31826 by Eric Snow in branch 'master': bpo-32604: Fix memory leaks in the new _xxsubinterpreters module. (bpo-5507) https://github.com/python/cpython/commit/4e9da0d163731caa79811c723c703ee416c31826
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.
New changeset f33ecedcad5a001735fa4ded5d21caa2cbf27911 by Eric Snow (Miss Islington (bot)) in branch '3.7': bpo-32604: Fix memory leaks in the new _xxsubinterpreters module. (GH-5507) https://github.com/python/cpython/commit/f33ecedcad5a001735fa4ded5d21caa2cbf27911
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. :)
New changeset 310b05289b5d9550040f469e60b5e8e77f1022b6 by Zachary Ware in branch 'master': bpo-32604: Make _xxsubinterpreters build on Windows (GH-5516) https://github.com/python/cpython/commit/310b05289b5d9550040f469e60b5e8e77f1022b6
New changeset fe61e8d8c7cd1f6fb0ce7e9b8f0460b47b5f29a5 by Zachary Ware (Miss Islington (bot)) in branch '3.7': bpo-32604: Make _xxsubinterpreters build on Windows (GH-5624) https://github.com/python/cpython/commit/fe61e8d8c7cd1f6fb0ce7e9b8f0460b47b5f29a5
New changeset 4c6955e2b0ccf88c705f8d1fac685a8e65f9699e by Eric Snow in branch 'master': bpo-32604: Clean up created subinterpreters before runtime finalization. (gh-5709) https://github.com/python/cpython/commit/4c6955e2b0ccf88c705f8d1fac685a8e65f9699e
New changeset 3db05a3a9c109f49d54b4965852e41c657e9b117 by Eric Snow (Miss Islington (bot)) in branch '3.7': bpo-32604: Clean up created subinterpreters before runtime finalization. (gh-5710) https://github.com/python/cpython/commit/3db05a3a9c109f49d54b4965852e41c657e9b117
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 ((bd093355…) $) » ./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
@Yury, thanks! I thought I had checked before I made the PR, but apparently not. I'll git it fixed.
I'm not seeing any refleak (on linux/clang). I'm guessing this is Windows-specific (based on use of "./python.exe"). How does test_multiprocessing_fork even run on Windows? I thought "fork" is an unsupported start method on Windows (see https://docs.python.org/3.7/library/multiprocessing.html#contexts-and-start-methods).
Also, I'm not sure how my change might cause a refleak outside of code using the _xxsubinterpreters module.
Davin, any thoughts on how my most recent commit (for this issue) might be causing the leaks Yury found?
Looks like macOS rather than Windows, and I can't reproduce it locally with current master:
14:28 $ ./python.exe -m test -R3:3 test_multiprocessing_fork Run tests sequentially 0:00:00 load avg: 2.68 [1/1] test_multiprocessing_fork beginning 6 repetitions 123456 ...... test_multiprocessing_fork passed in 9 min 20 sec 1 test OK.
Total duration: 9 min 20 sec Tests result: SUCCESS
The Windows and Linux refleak buildbots [1] are currently unhappy about other things that appear to be unrelated.
[1] http://buildbot.python.org/all/#/builders?tags=%2Brefleak
FYI I found out about this refleak from https://mail.python.org/pipermail/python-checkins/2018-February/153907.html
So it's definitely not Mac OS X specific thing.