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

Py_NewInterpreter does not work #47973

Closed
amauryfa opened this issue Aug 29, 2008 · 17 comments
Closed

Py_NewInterpreter does not work #47973

amauryfa opened this issue Aug 29, 2008 · 17 comments
Assignees
Labels
extension-modules C modules in the Modules dir release-blocker type-bug An unexpected behavior, bug, or error

Comments

@amauryfa
Copy link
Member

BPO 3723
Nosy @loewis, @warsaw, @amauryfa, @tiran, @benjaminp, @djc
Dependencies
  • bpo-4213: Lower case file system encoding
  • Files
  • add_init_funcs.patch
  • pythonrun.c.diff
  • importexc.diff
  • subinterpreter.patch
  • 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/warsaw'
    closed_at = <Date 2008-10-30.21:48:53.355>
    created_at = <Date 2008-08-29.12:11:09.887>
    labels = ['extension-modules', 'type-bug', 'release-blocker']
    title = 'Py_NewInterpreter does not work'
    updated_at = <Date 2008-10-30.21:48:53.353>
    user = 'https://github.com/amauryfa'

    bugs.python.org fields:

    activity = <Date 2008-10-30.21:48:53.353>
    actor = 'christian.heimes'
    assignee = 'barry'
    closed = True
    closed_date = <Date 2008-10-30.21:48:53.355>
    closer = 'christian.heimes'
    components = ['Extension Modules']
    creation = <Date 2008-08-29.12:11:09.887>
    creator = 'amaury.forgeotdarc'
    dependencies = ['4213']
    files = ['11524', '11661', '11820', '11895']
    hgrepos = []
    issue_num = 3723
    keywords = ['patch', 'needs review']
    message_count = 17.0
    messages = ['72127', '73403', '73530', '73531', '74069', '74070', '74071', '74072', '74919', '74938', '74939', '74940', '74943', '75248', '75277', '75338', '75385']
    nosy_count = 7.0
    nosy_names = ['loewis', 'barry', 'amaury.forgeotdarc', 'christian.heimes', 'benjamin.peterson', 'djc', 'grahamd']
    pr_nums = []
    priority = 'release blocker'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue3723'
    versions = ['Python 3.0']

    @amauryfa
    Copy link
    Member Author

    The example Demo/embed/importexc.c crashes, because Py_NewInterpreter
    cannot reimport builtins and sys modules. This problem seems important
    for embedding applications like mod_python, for example.

    (the "import exceptions" statement does not work with python 3.0, but
    replacing with e.g. "import types" does not change anything: the
    interpreter is not correctly renewed)

    I tried to put "-1" in the m_size structure of these modules, and they
    seem to import correctly. However, "builtins" comes with its original
    dictionary - without the standard exceptions.

    I think that these modules should be made re-importable, with specific
    functions.

    Maybe two related problems:

    • once you've done
      del sys.modules['sys']
      it's not possible to get it back, "import sys" raises an error...

    • the usual trick to call sys.setdefaultencoding will not work, since it
      needs to "imp.reload(sys)"

    @benjaminp
    Copy link
    Contributor

    Maybe, I'm not seeing the whole problem, but can't we just add
    _PySys_Init and _PyBuiltin_Init to config.c like in the attached patch?
    Obviously, we will eventually want to make a separate state to store
    module globals in, but I think this will work for 3.0 final.

    @amauryfa
    Copy link
    Member Author

    I applied the patch to PC/config.c, but this did not change anything.

    @benjaminp
    Copy link
    Contributor

    Interesting, here it lets import.c's init_builtin reinitalize modules...

    @grahamd
    Copy link
    Mannequin

    grahamd mannequin commented Sep 30, 2008

    Adding the functions as initfunc in module init table is of no use as
    they aren't invoked when creating a sub interpreter.

    One thing that does appear to work, although no idea of whether it is
    correct way to solve problem, is to duplicate the builtin/sys
    initialisation that occurs in Py_InitializeEx() function.

    Attached diff shows nature of changes. Diff is bit messy as have left
    existing code in there but #ifdef'd out.

    Maybe this will give someone who knows how overall interpreter
    initialisation is supposed to work a head start on coming up with proper
    fix. But then it could be totally wrong as well.

    At least with change as is, mod_wsgi works for sub interpreters now.
    I'll do more work later on whether it is correct way to solve it.

    @amauryfa
    Copy link
    Member Author

    Your patch may go in the right direction, but please provide only
    context diff or unified diff files.
    Use "diff -du", or "svn diff" to generate the file.

    @grahamd
    Copy link
    Mannequin

    grahamd mannequin commented Sep 30, 2008

    Argh. Personally I like to provide context diff's but more often than not
    get abused for providing them over a unified diff. Was in a hurry this
    time as had only a couple of minutes of battery life left on the laptop,
    so quickly did it without thinking and then ran off to find a power point.
    :-)

    @grahamd
    Copy link
    Mannequin

    grahamd mannequin commented Sep 30, 2008

    Unified diff now attached.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Oct 17, 2008

    Here is a patch that fixed importexc.c. It consists of the following parts:

    • set m_size of the builtins module and the sys module to -1, indicating
      that these modules don't support repeated initialization. This should be
      reviewed; perhaps it's better (and necessary) to record the init
      function not only for dynamically-loaded modules in m_init, but also for
      statically linked ones, so that the reinit code can call them again
      (whether or not it is safe to call them again for sys and builtins
      should then be studied).
    • add a field to the interpreter state indicating that the codecs are
      not ready. when trying to use the file system encoding in this state,
      use ASCII instead.
    • Fix importexc to use the types module.

    @amauryfa
    Copy link
    Member Author

    I think the patch goes in the right direction.

    But in addition, Py_NewInterpreter() has to call initstdio() between initmain() and initsite() (the same sequence as in Py_InitializeEx)
    Found by using the following command string in importexc.c:
    "import types; print(types.XXX)"

    @benjaminp
    Copy link
    Contributor

    Wouldn't it make more sense to move interpreter initialization things to
    Py_NewInterpreter and call it from Py_InitializeEx?

    @tiran
    Copy link
    Member

    tiran commented Oct 17, 2008

    Sounds like a good plan, Benjamin

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Oct 17, 2008

    Wouldn't it make more sense to move interpreter initialization things to
    Py_NewInterpreter and call it from Py_InitializeEx?

    Can you propose a specific patch? I'm worried that doing so blindly
    introduces other bugs.

    @tiran
    Copy link
    Member

    tiran commented Oct 26, 2008

    The patch "subinterpreter.patch" is based on Martin's patch
    "importexc.diff". The patch contains additional code to setup a
    preliminary stderr object and a call to initstdio(). Amaury is right. I
    had to add initstdio() to initialize the standard streams. But I can't
    get it to work.

    $ rm -f Demo/embed/importexc.o; cd Demo/embed; make; ./importexc; cd ../..
    gcc -g -I../../Include -I../..  -c -o importexc.o importexc.c
    gcc -Xlinker -export-dynamic importexc.o ../../libpython3.0.a  -lnsl
    -ldl -lreadline -ltermcap -lieee -lpthread -lutil -lm -o importexc
    Initialize interpreter
    <module 'types' from '/usr/local/lib/python3.0/types.py'>
    Initialize subinterpreter
    Fatal Python error: Py_Initialize: can't initialize sys standard streams
    Traceback (most recent call last):
      File "/usr/local/lib/python3.0/encodings/__init__.py", line 32, in
    <module>
    ValueError: Cannot encode path item
    Aborted

    @tiran
    Copy link
    Member

    tiran commented Oct 28, 2008

    In combination with the patch in bpo-4213, "subinterpreter.patch" fixes the
    problem.

    I'm assigning the bug to Barry for his final decision.

    @tiran tiran added the type-bug An unexpected behavior, bug, or error label Oct 28, 2008
    @grahamd
    Copy link
    Mannequin

    grahamd mannequin commented Oct 30, 2008

    In conjunction with bpo-4213, the attached subinterpreter.patch appears to
    fix issue for mod_wsgi.

    @tiran
    Copy link
    Member

    tiran commented Oct 30, 2008

    Applied in r67057

    @tiran tiran closed this as completed Oct 30, 2008
    @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
    Labels
    extension-modules C modules in the Modules dir release-blocker type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants