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.

Sign up for GitHub

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

Jump to bottom

new_interpreter() should reuse more Py_InitializeFromConfig() code #83039

Closed
vstinner opened this issue Nov 19, 2019 · 23 comments
Closed

new_interpreter() should reuse more Py_InitializeFromConfig() code #83039

vstinner opened this issue Nov 19, 2019 · 23 comments
Labels
3.9 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-subinterpreters

Comments

@vstinner
Copy link
Member

BPO 38858
Nosy @vstinner, @ericsnowcurrently
PRs
  • bpo-38858: Reorganize pycore_init_types() #17265
  • bpo-38858: Factorize Py_EndInterpreter() code #17273
  • bpo-38858: Fix reference leak in pycore_init_types() #17286
  • bpo-38858: Add _Py_IsMainInterpreter(tstate) #17293
  • bpo-38858: Fix Py_Finalize() when called from a subinterpreter #17297
  • bpo-38858: Allocate small integers on the heap #17301
  • bpo-38858: Small integer per interpreter #17315
  • bpo-38858: Call _PyUnicode_Fini() in Py_EndInterpreter() #17330
  • bpo-38858: Add init_set_builtins_open() subfunction #17346
  • bpo-38858: Add init_interp_main() subfunction #17347
  • bpo-38858: _PyImport_FixupExtensionObject() handles subinterpreters #17350
  • bpo-38858: new_interpreter() reuses pycore_init_builtins() #17351
  • bpo-38858: new_interpreter() uses pycore_init_import_warnings() #17353
  • bpo-38858: new_interpreter() reuses _PySys_Create() #17481
  • bpo-38858: Add pycore_interp_init() code to factorize code #17483
  • bpo-38858: Fix ref leak in pycore_interp_init() #17512
    		•

    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 = <Date 2019-12-06.16:01:54.899>
created_at = <Date 2019-11-19.23:11:09.543>
labels = ['expert-subinterpreters', 'interpreter-core', '3.9']
title = 'new_interpreter() should reuse more Py_InitializeFromConfig() code'
updated_at = <Date 2020-05-15.00:37:42.264>
user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2020-05-15.00:37:42.264>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2019-12-06.16:01:54.899>
closer = 'vstinner'
components = ['Interpreter Core', 'Subinterpreters']
creation = <Date 2019-11-19.23:11:09.543>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 38858
keywords = ['patch']
message_count = 23.0
messages = ['357001', '357005', '357043', '357052', '357055', '357083', '357092', '357139', '357140', '357268', '357278', '357296', '357298', '357306', '357311', '357321', '357897', '357899', '357920', '358028', '358356', '358543', '360057']
nosy_count = 2.0
nosy_names = ['vstinner', 'eric.snow']
pr_nums = ['17265', '17273', '17286', '17293', '17297', '17301', '17315', '17330', '17346', '17347', '17350', '17351', '17353', '17481', '17483', '17512']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue38858'
versions = ['Python 3.9']
    @vstinner
    Copy link
    Member Author

    Currently, new_interpreter() is a subset of Py_InitializeFromConfig(): the code was duplicated. I would prefer that both functions stay in sync and so that new_interpreter() reuses more Py_InitializeFromConfig() code.

    @vstinner vstinner added 3.9 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Nov 19, 2019
    @vstinner
    Copy link
    Member Author

    New changeset ef5aa9a by Victor Stinner in branch 'master':
    bpo-38858: Reorganize pycore_init_types() (GH-17265)
    ef5aa9a

    @vstinner
    Copy link
    Member Author

    New changeset 7eee5be by Victor Stinner in branch 'master':
    bpo-38858: Factorize Py_EndInterpreter() code (GH-17273)
    7eee5be

    @vstinner
    Copy link
    Member Author

    New changeset ef5aa9a by Victor Stinner in branch 'master':
    bpo-38858: Reorganize pycore_init_types() (GH-17265)

    This change introduced a reference leak:

    https://buildbot.python.org/all/#builders/80/builds/771

    test_atexit leaked [792, 792, 792] references, sum=2376
    test_capi leaked [528, 528, 528] references, sum=1584
    test__xxsubinterpreters leaked [17954, 17952, 17954] references, sum=53860
    test_threading leaked [792, 792, 792] references, sum=2376

    It should be fixed by PR 17286.

    @vstinner
    Copy link
    Member Author

    New changeset e7e699e by Victor Stinner in branch 'master':
    bpo-38858: Fix reference leak in pycore_init_types() (GH-17286)
    e7e699e

    @vstinner
    Copy link
    Member Author

    New changeset fff7bbf by Victor Stinner in branch 'master':
    bpo-38858: Add _Py_IsMainInterpreter(tstate) (GH-17293)
    fff7bbf

    @vstinner
    Copy link
    Member Author

    New changeset b93f31f by Victor Stinner in branch 'master':
    bpo-38858: Fix Py_Finalize() when called from a subinterpreter (GH-17297)
    b93f31f

    @vstinner
    Copy link
    Member Author

    New changeset 5dcc06f by Victor Stinner in branch 'master':
    bpo-38858: Allocate small integers on the heap (GH-17301)
    5dcc06f

    @vstinner
    Copy link
    Member Author

    bpo-38858: Allocate small integers on the heap (GH-17301)

    I associated this change to this issue because I had troubles when I tried to call _PyLong_Fini() in subinterpreters. I'm now trying to have different small integers per interpreter.

    @vstinner
    Copy link
    Member Author

    New changeset 3d48334 by Victor Stinner in branch 'master':
    bpo-38858: Call _PyUnicode_Fini() in Py_EndInterpreter() (GH-17330)
    3d48334

    @vstinner
    Copy link
    Member Author

    I introduced a workaround to a reference leak in bpo-36854 because _PyImport_Cleanup() doesn't clear modules dictionary in some cases:
    https://bugs.python.org/issue36854#msg357160

    Maybe _PyImport_Cleanup() should be enhanced to ensure that the dictionary of all modules is cleared when the function exit.

    @vstinner
    Copy link
    Member Author

    New changeset e0c9ab8 by Victor Stinner in branch 'master':
    bpo-38858: Add init_set_builtins_open() subfunction (GH-17346)
    e0c9ab8

    @vstinner
    Copy link
    Member Author

    New changeset b005136 by Victor Stinner in branch 'master':
    bpo-38858: Add init_interp_main() subfunction (GH-17347)
    b005136

    @vstinner
    Copy link
    Member Author

    New changeset 82c83bd by Victor Stinner in branch 'master':
    bpo-38858: _PyImport_FixupExtensionObject() handles subinterpreters (GH-17350)
    82c83bd

    @vstinner
    Copy link
    Member Author

    New changeset 2582d46 by Victor Stinner in branch 'master':
    bpo-38858: new_interpreter() reuses pycore_init_builtins() (GH-17351)
    2582d46

    @vstinner
    Copy link
    Member Author

    New changeset 2ec1a1b by Victor Stinner in branch 'master':
    bpo-38858: new_interpreter() uses pycore_init_import_warnings() (GH-17353)
    2ec1a1b

    @vstinner
    Copy link
    Member Author

    vstinner commented Dec 6, 2019

    New changeset 81fe5bd by Victor Stinner in branch 'master':
    bpo-38858: new_interpreter() reuses _PySys_Create() (GH-17481)
    81fe5bd

    @vstinner
    Copy link
    Member Author

    vstinner commented Dec 6, 2019

    New changeset d863ade by Victor Stinner in branch 'master':
    bpo-38858: Add pycore_interp_init() code to factorize code (GH-17483)
    d863ade

    @vstinner
    Copy link
    Member Author

    vstinner commented Dec 6, 2019

    Py_NewInterpreter() (new_interpreter() in practice) and Py_InitializeEx() now share almost all their code.

    Py_EndInterpreter() shares almost all its code with Py_Finalizer(). It's not perfect, but it's way better than previously.

    Py_NewInterpreter() now isolates more things from the main interpreter. For example, builtins and sys modules no longer copy the module dictionary of the main interpreter, but create their own dictionary from scratch.

    See each commit for the details.

    Py_Finalizer() could share more code with Py_EndInterpreter(), but each Py_Finalizer() change is really tricky and require to pay a lot attention. The Python finalization is really fragile. I started to take notes on this fragile code:
    https://pythondev.readthedocs.io/finalization.html

    I consider that the initial issue is fixed, so I close the issue.

    @vstinner vstinner closed this as completed Dec 6, 2019
    @vstinner
    Copy link
    Member Author

    vstinner commented Dec 8, 2019

    New changeset 080ee5a by Victor Stinner in branch 'master':
    bpo-38858: Fix ref leak in pycore_interp_init() (GH-17512)
    080ee5a

    @ericsnowcurrently
    Copy link
    Member

    Thanks for working on this. It really does have far-reaching benefits, not just for the subinterpreter stuff I'm interested in. :)

    @vstinner
    Copy link
    Member Author

    New changeset 630c8df by Victor Stinner in branch 'master':
    bpo-38858: Small integer per interpreter (GH-17315)
    630c8df

    @vstinner
    Copy link
    Member Author

    FYI this change broke librepo which calls PyLong_FromLong() without holding the GIL. In Python 3.8, "it works". In Python 3.9, it does crash: get_small_int() gets a NULL tstate and then dereference a NULL pointer.

    librepo bug:
    https://bugzilla.redhat.com/show_bug.cgi?id=1788918

    IMHO it's a bug in librepo: the GIL must be held to use Python C API.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.9 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-subinterpreters
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @vstinner @ericsnowcurrently