classification
Title: Expose the subinterpreters C-API in Python for testing use.
Type: enhancement Stage: patch review
Components: Interpreter Core Versions: Python 3.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: eric.snow Nosy List: davin, eric.snow, ncoghlan, ned.deily, pitrou, pmpp, steve.dower, yselivanov, zach.ware
Priority: release blocker Keywords: patch

Created on 2018-01-20 01:58 by eric.snow, last changed 2018-02-21 00:09 by eric.snow.

Pull Requests
URL Status Linked Edit
PR 1748 merged eric.snow, 2018-01-20 02:06
PR 5436 open eric.snow, 2018-01-30 01:30
PR 5437 merged eric.snow, 2018-01-30 03:29
PR 5507 merged eric.snow, 2018-02-03 04:27
PR 5509 merged miss-islington, 2018-02-03 04:53
PR 5516 merged zach.ware, 2018-02-03 20:23
PR 5624 merged miss-islington, 2018-02-11 16:44
PR 5709 merged eric.snow, 2018-02-16 23:31
PR 5710 merged miss-islington, 2018-02-17 01:54
PR 5778 merged eric.snow, 2018-02-20 17:46
PR 5783 merged miss-islington, 2018-02-20 23:31
Messages (33)
msg310314 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2018-01-20 01:58
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 #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. :)
msg310316 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2018-01-20 02:03
@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?
msg310317 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2018-01-20 02:04
@Nick, I may make the name change you suggested in issue #30439 ("_subinterpreters").
msg310318 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2018-01-20 02:10
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. :)
msg310448 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2018-01-22 22:46
@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.
msg310562 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2018-01-24 05:38
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.
msg311210 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2018-01-30 01:23
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
msg311211 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2018-01-30 01:33
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.
msg311212 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2018-01-30 01:47
Eric, looks like some buildbots are unhappy, for instance:

http://buildbot.python.org/all/#builders/13/builds/648
msg311217 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2018-01-30 03:19
Yeah, I'm looking into it.  Also, I noticed some refleaks that I'll be sorting out.
msg311218 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2018-01-30 03:23
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)
msg311219 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2018-01-30 03:23
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)
?           +
msg311220 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2018-01-30 03:30
I just put up a PR that should fix the 4 buildbots.
msg311222 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2018-01-30 04:04
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
msg311224 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2018-01-30 04:04
The buildbots should be happier now.  I'll keep an eye on them.
msg311299 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-01-30 20:30
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()
msg311533 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2018-02-03 04:49
New changeset 4e9da0d163731caa79811c723c703ee416c31826 by Eric Snow in branch 'master':
bpo-32604: Fix memory leaks in the new _xxsubinterpreters module. (#5507)
https://github.com/python/cpython/commit/4e9da0d163731caa79811c723c703ee416c31826
msg311534 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2018-02-03 04:51
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.
msg311536 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2018-02-03 05:39
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
msg311538 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2018-02-03 06:00
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. :)
msg312001 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2018-02-11 16:43
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
msg312004 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2018-02-11 17:16
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
msg312260 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2018-02-17 01:53
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
msg312262 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2018-02-17 02:15
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
msg312288 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-02-17 21:12
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
msg312362 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2018-02-19 19:26
@Yury, thanks!  I thought I had checked before I made the PR, but apparently not.  I'll git it fixed.
msg312367 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2018-02-19 20:20
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.
msg312368 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2018-02-19 20:23
Davin, any thoughts on how my most recent commit (for this issue) might be causing the leaks Yury found?
msg312369 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2018-02-19 20:45
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
msg312370 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-02-19 20:48
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.
msg312372 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2018-02-19 20:49
I'm pretty sure that report is still running on the last changeset on hg.python.org; can you confirm that Antoine?
msg312443 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2018-02-20 23:30
New changeset f53d9f2778a87bdd48eb9030f782a4ebf9e7622f by Eric Snow in branch 'master':
bpo-32604: Swap threads only if the interpreter is different. (gh-5778)
https://github.com/python/cpython/commit/f53d9f2778a87bdd48eb9030f782a4ebf9e7622f
msg312447 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2018-02-21 00:09
New changeset eed3c7af4ec520d324a55e0357f41589a66906f7 by Eric Snow (Miss Islington (bot)) in branch '3.7':
bpo-32604: Swap threads only if the interpreter is different. (gh-5783)
https://github.com/python/cpython/commit/eed3c7af4ec520d324a55e0357f41589a66906f7
History
Date User Action Args
2018-02-21 00:09:44eric.snowsetmessages: + msg312447
2018-02-20 23:31:27miss-islingtonsetpull_requests: + pull_request5564
2018-02-20 23:30:25eric.snowsetmessages: + msg312443
2018-02-20 17:46:06eric.snowsetpull_requests: + pull_request5559
2018-02-19 21:19:34pmppsetnosy: + pmpp
2018-02-19 20:49:59zach.waresetnosy: + pitrou
messages: + msg312372
2018-02-19 20:48:48yselivanovsetmessages: + msg312370
2018-02-19 20:45:24zach.waresetmessages: + msg312369
2018-02-19 20:23:22eric.snowsetnosy: + davin
messages: + msg312368
2018-02-19 20:20:23eric.snowsetmessages: + msg312367
2018-02-19 19:26:48eric.snowsetmessages: + msg312362
2018-02-17 21:12:20yselivanovsetpriority: normal -> release blocker

messages: + msg312288
2018-02-17 02:15:26eric.snowsetmessages: + msg312262
2018-02-17 01:54:23miss-islingtonsetpull_requests: + pull_request5499
2018-02-17 01:53:48eric.snowsetmessages: + msg312260
2018-02-16 23:31:42eric.snowsetpull_requests: + pull_request5498
2018-02-11 17:16:06zach.waresetmessages: + msg312004
2018-02-11 16:44:58miss-islingtonsetpull_requests: + pull_request5432
2018-02-11 16:43:50zach.waresetnosy: + zach.ware
messages: + msg312001
2018-02-03 20:23:42zach.waresetpull_requests: + pull_request5348
2018-02-03 06:00:20eric.snowsetmessages: + msg311538
2018-02-03 05:39:00eric.snowsetmessages: + msg311536
2018-02-03 04:53:20miss-islingtonsetpull_requests: + pull_request5340
2018-02-03 04:51:07eric.snowsetmessages: + msg311534
2018-02-03 04:49:51eric.snowsetmessages: + msg311533
2018-02-03 04:27:04eric.snowsetpull_requests: + pull_request5338
2018-01-30 20:30:49yselivanovsetmessages: + msg311299
2018-01-30 04:04:48eric.snowsetmessages: + msg311224
2018-01-30 04:04:17eric.snowsetmessages: + msg311222
2018-01-30 03:30:08eric.snowsetmessages: + msg311220
2018-01-30 03:29:29eric.snowsetpull_requests: + pull_request5270
2018-01-30 03:23:39eric.snowsetmessages: + msg311219
2018-01-30 03:23:05eric.snowsetmessages: + msg311218
2018-01-30 03:19:39eric.snowsetmessages: + msg311217
2018-01-30 01:47:45ned.deilysetmessages: + msg311212
2018-01-30 01:33:02eric.snowsetmessages: + msg311211
2018-01-30 01:30:54eric.snowsetpull_requests: + pull_request5269
2018-01-30 01:23:46eric.snowsetmessages: + msg311210
2018-01-24 05:38:28eric.snowsetmessages: + msg310562
2018-01-22 22:46:19ned.deilysetmessages: + msg310448
2018-01-20 02:10:10eric.snowsetmessages: + msg310318
2018-01-20 02:06:25eric.snowsetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request5094
2018-01-20 02:04:36eric.snowsetmessages: + msg310317
2018-01-20 02:03:42eric.snowsetnosy: + ned.deily, steve.dower
messages: + msg310316
2018-01-20 01:58:02eric.snowcreate