Skip to content
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.

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

[subinterpreters] PEP 554 implementation: add interpreters module #76785

Open
ericsnowcurrently opened this issue Jan 20, 2018 · 66 comments
Open
Assignees
Labels
3.10 only security fixes docs Documentation in the Doc dir topic-subinterpreters type-feature A feature request or enhancement

Comments

@ericsnowcurrently
Copy link
Member

ericsnowcurrently commented Jan 20, 2018

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']

    Linked PRs

    @ericsnowcurrently
    Copy link
    Member Author

    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 ericsnowcurrently added 3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Jan 20, 2018
    @ericsnowcurrently ericsnowcurrently self-assigned this Jan 20, 2018
    @ericsnowcurrently ericsnowcurrently added the type-feature A feature request or enhancement label Jan 20, 2018
    @ericsnowcurrently
    Copy link
    Member Author

    @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
    Copy link
    Member Author

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

    @ericsnowcurrently
    Copy link
    Member Author

    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
    Copy link
    Member

    @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
    Copy link
    Member Author

    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
    Copy link
    Member Author

    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
    Copy link
    Member Author

    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
    Copy link
    Member

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

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

    @ericsnowcurrently
    Copy link
    Member Author

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

    @ericsnowcurrently
    Copy link
    Member Author

    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
    Copy link
    Member Author

    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
    Copy link
    Member Author

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

    @ericsnowcurrently
    Copy link
    Member Author

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

    @ericsnowcurrently
    Copy link
    Member Author

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

    @1st1
    Copy link
    Member

    1st1 commented Jan 30, 2018

    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
    Copy link
    Member Author

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

    @ericsnowcurrently
    Copy link
    Member Author

    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
    Copy link
    Member Author

    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
    Copy link
    Member Author

    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
    Copy link
    Member

    zware commented Feb 11, 2018

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

    @zware
    Copy link
    Member

    zware commented Feb 11, 2018

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

    @ericsnowcurrently
    Copy link
    Member Author

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

    @ericsnowcurrently
    Copy link
    Member Author

    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
    Copy link
    Member

    1st1 commented Feb 17, 2018

    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 added a commit to ericsnowcurrently/cpython that referenced this issue Nov 6, 2023
    hugovk pushed a commit to hugovk/cpython that referenced this issue Nov 8, 2023
    …ythongh-111715)
    
    I added _Py_excinfo to the internal API (and added its functions in Python/errors.c) in pythongh-111530 (9322ce9).  Since then I've had a nagging sense that I should have added the type and functions in its own PR.  While I do plan on using _Py_excinfo outside crossinterp.c very soon (see pythongh-111572/pythongh-111573), I'd still feel more comfortable if the _Py_excinfo stuff went in as its own PR.  Hence, here we are.
    
    (FWIW, I may combine that with pythongh-111572, which I may, in turn, combine with pythongh-111573.  We'll see.)
    ericsnowcurrently added a commit that referenced this issue Nov 22, 2023
    The new function corresponds to the existing (public) PyType_GetName() and PyType_GetQualName().
    ericsnowcurrently added a commit that referenced this issue Nov 27, 2023
    …ata (gh-109556) (gh-112288)
    
    This fixes some crashes in the _xxinterpchannels module, due to a race between interpreters.
    (cherry picked from commit fd7e08a)
    ericsnowcurrently added a commit that referenced this issue Dec 12, 2023
    This involves a number of changes for PEP 734.
    ericsnowcurrently added a commit that referenced this issue Dec 12, 2023
    gh-112982 broke test_threading on one of the s390 buildbots (Fedora Clang Installed).  Apparently ImportError is raised (rather than ModuleNotFoundError) for the name part of "from" imports.  This fixes that by catching ImportError in test_threading.py.
    ericsnowcurrently added a commit that referenced this issue Dec 12, 2023
    This brings the module (along with the associated extension modules) mostly in sync with PEP 734.  There are only a few small things to wrap up.
    ericsnowcurrently added a commit that referenced this issue Dec 12, 2023
    This is one of the last pieces to get test.support.interpreters in sync with PEP 734.
    ericsnowcurrently added a commit that referenced this issue Dec 13, 2023
    …h-113034)
    
    When an exception is uncaught in Interpreter.exec_sync(), it helps to show that exception's error display if uncaught in the calling interpreter.  We do so here by generating a TracebackException in the subinterpreter and passing it between interpreters using pickle.
    ericsnowcurrently added a commit that referenced this issue Dec 13, 2023
    …ter Exceptions (gh-113036)
    
    We need the TracebackException of uncaught exceptions for a single purpose: the error display.  Thus we only need to pass the formatted error display between interpreters.  Passing a pickled TracebackException is overkill.
    ericsnowcurrently added a commit that referenced this issue Dec 13, 2023
    In gh-112982 I made some changes to .github/CODEOWNERS.  Later, @ezio-melotti pointed out that some of those changes were unnecessary.
    jeff5 added a commit to jeff5/cpython that referenced this issue Dec 25, 2023
    …GH-112982)
    
    This is so that we can run python -m test.test_interpreters. As such it
    backports that aspect of commit 86a77f4,
    where it is implemnented by a package __main__.py.
    jeff5 added a commit to jeff5/cpython that referenced this issue Dec 25, 2023
    …GH-112982)
    
    This is so that we can run python -m test.test_interpreters. As such it
    backports that aspect of commit 86a77f4,
    where it is implemented by a package __main__.py.
    serhiy-storchaka pushed a commit that referenced this issue Dec 25, 2023
    …H-113470)
    
    This is so that we can run python -m test.test_interpreters. As such it
    backports that aspect of commit 86a77f4,
    where it is implemented by a package __main__.py.
    miss-islington pushed a commit to miss-islington/cpython that referenced this issue Dec 25, 2023
    …GH-112982) (pythonGH-113470)
    
    This is so that we can run python -m test.test_interpreters. As such it
    backports that aspect of commit 86a77f4,
    where it is implemented by a package __main__.py.
    (cherry picked from commit bdad5c3)
    
    Co-authored-by: Jeff Allen <ja.py@farowl.co.uk>
    serhiy-storchaka pushed a commit that referenced this issue Dec 25, 2023
    …H-113470) (GH-113473)
    
    This is so that we can run python -m test.test_interpreters. As such it
    backports that aspect of commit 86a77f4,
    where it is implemented by a package __main__.py.
    (cherry picked from commit bdad5c3)
    
    Co-authored-by: Jeff Allen <ja.py@farowl.co.uk>
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.10 only security fixes docs Documentation in the Doc dir topic-subinterpreters type-feature A feature request or enhancement
    Projects
    Status: Todo
    Development

    No branches or pull requests

    9 participants