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

Frozen stdlib modules are discarded if custom frozen modules added. #89558

Closed
ericsnowcurrently opened this issue Oct 6, 2021 · 21 comments
Closed
Labels
3.11 bug and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@ericsnowcurrently
Copy link
Member

BPO 45395
Nosy @malemburg, @brettcannon, @ericsnowcurrently, @corona10
PRs
  • bpo-45395: Make custom frozen modules additions instead of replacements. #28778
  • 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 2021-10-28.21:34:11.618>
    created_at = <Date 2021-10-06.20:21:03.474>
    labels = ['interpreter-core', 'type-bug', '3.11']
    title = 'Frozen stdlib modules are discarded if custom frozen modules added.'
    updated_at = <Date 2021-10-28.21:34:11.618>
    user = 'https://github.com/ericsnowcurrently'

    bugs.python.org fields:

    activity = <Date 2021-10-28.21:34:11.618>
    actor = 'eric.snow'
    assignee = 'none'
    closed = True
    closed_date = <Date 2021-10-28.21:34:11.618>
    closer = 'eric.snow'
    components = ['Interpreter Core']
    creation = <Date 2021-10-06.20:21:03.474>
    creator = 'eric.snow'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 45395
    keywords = ['patch']
    message_count = 21.0
    messages = ['403335', '403339', '403353', '403420', '403421', '403946', '404006', '404059', '404070', '404112', '404316', '404343', '404432', '404462', '404467', '404476', '404698', '405042', '405044', '405268', '405278']
    nosy_count = 4.0
    nosy_names = ['lemburg', 'brett.cannon', 'eric.snow', 'corona10']
    pr_nums = ['28778']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue45395'
    versions = ['Python 3.11']

    @ericsnowcurrently
    Copy link
    Member Author

    The mechanism to add custom frozen modules to the Python runtime is to set PyImport_FrozenModules (see Python/frozen.c) to some new array. This means that the default frozen modules (from _PyImport_FrozenModules) are no longer used unless explicitly added to the custom array. This is unlikely to be what the user wants. It's especially problematic since it's easy to not realize this (or forget) and forgetting essential modules (like _frozen_importlib) will cause crashes.

    It would probably make more sense to have PyImport_FrozenModules be an array of *additional* frozen modules, defaulting to an empty array. Before going down that route we need to be sure that isn't going to break folks that are accommodating the existing behavior.

    @ericsnowcurrently ericsnowcurrently added 3.11 bug and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error labels Oct 6, 2021
    @ericsnowcurrently
    Copy link
    Member Author

    I've posted a PR that demonstrates a reasonable solution.

    @malemburg
    Copy link
    Member

    I'm not sure I follow, but in any case, please make sure that
    the freeze tool in Tools/ continues to work with the new mechanism.

    The freeze tool would also need to know which modules are already
    frozen via the new script, so that modules don't get included twice.

    @ericsnowcurrently
    Copy link
    Member Author

    On Thu, Oct 7, 2021 at 1:17 AM Marc-Andre Lemburg
    <report@bugs.python.org> wrote:

    I'm not sure I follow, but in any case, please make sure that
    the freeze tool in Tools/ continues to work with the new mechanism.

    The freeze tool would also need to know which modules are already
    frozen via the new script, so that modules don't get included twice.

    Will do.

    @malemburg
    Copy link
    Member

    On 07.10.2021 16:40, Eric Snow wrote:

    On Thu, Oct 7, 2021 at 1:17 AM Marc-Andre Lemburg
    <report@bugs.python.org> wrote:
    > I'm not sure I follow, but in any case, please make sure that
    > the freeze tool in Tools/ continues to work with the new mechanism.
    >
    > The freeze tool would also need to know which modules are already
    > frozen via the new script, so that modules don't get included twice.

    Will do.

    Great, thanks, Eric.

    Marc-Andre Lemburg
    eGenix.com

    @ericsnowcurrently
    Copy link
    Member Author

    @mal, what's the best way to make sure Tools/freeze is still working? I don't see any tests for it in the test suite. I tried running the test in Tools/freeze/test, but I can't get that to work on main (or on the 3.10 branch).

    @malemburg
    Copy link
    Member

    On 14.10.2021 22:56, Eric Snow wrote:

    @mal, what's the best way to make sure Tools/freeze is still working? I don't see any tests for it in the test suite. I tried running the test in Tools/freeze/test, but I can't get that to work on main (or on the 3.10 branch).

    You'd have to create a frozen binary using the standard way freeze
    works. I have never run those tests, so don't know whether they work,
    but, of course, made sure that the freeze works as basis for PyRun
    and patched it slightly to add features we needed.

    One of these days, I need to refactor PyRun into a standalone project
    and put it on Github (it's currently integrated into our internal
    single repo setup). Then it'll be easier to see the changes I made.
    For now, I can only reference the tar file:

    https://www.egenix.com/products/python/PyRun/#Download
    https://www.egenix.com/products/python/PyRun/#Installation

    I can send you an updated version for Python 3.8, if there's
    interest.

    Essentially, you need to create a Python module which runs your
    application, then point freeze.py to it and then compile the
    generated .c files using the generated Makefile.

    @ericsnowcurrently
    Copy link
    Member Author

    @mal, who's maintaining Tools/freeze? I'm not aware of who's using it (other than you, of course). It looks like PyRun isn't compatible with anything newer than 3.5, so it seems like that isn't verifying that Tools/freeze still works. Neither does it have tests that run in the test suite (nor on buildbots).

    So could Tools/freeze have been broken for a while? I ask because I haven't been able to get it work work on the master branch (or on 3.10).

    @malemburg
    Copy link
    Member

    On 16.10.2021 01:31, Eric Snow wrote:

    @mal, who's maintaining Tools/freeze? I'm not aware of who's using it (other than you, of course). It looks like PyRun isn't compatible with anything newer than 3.5, so it seems like that isn't verifying that Tools/freeze still works. Neither does it have tests that run in the test suite (nor on buildbots).

    So could Tools/freeze have been broken for a while? I ask because I haven't been able to get it work work on the master branch (or on 3.10).

    I don't know who maintains it, but it's been working fine up until Python 3.8,
    which is the last version I ported PyRun to.

    There have also been a couple of patches going into the freeze tool, so this is
    still on the radar of at least some people other than me. It's also one of the
    oldest tools we have in Python and dates back to the early days of Python. Guido
    wrote the initial version.

    I can try to port PyRun to 3.9 and 3.10 to see whether I run into any issues.
    Would that help ?

    @ericsnowcurrently
    Copy link
    Member Author

    On Sat, Oct 16, 2021 at 5:01 AM Marc-Andre Lemburg
    <report@bugs.python.org> wrote:

    I can try to port PyRun to 3.9 and 3.10 to see whether I run into any issues.
    Would that help ?

    Yeah, that would totally help.

    @malemburg
    Copy link
    Member

    On 16.10.2021 21:20, Eric Snow wrote:

    On Sat, Oct 16, 2021 at 5:01 AM Marc-Andre Lemburg
    <report@bugs.python.org> wrote:
    > I can try to port PyRun to 3.9 and 3.10 to see whether I run into any issues.
    > Would that help ?

    Yeah, that would totally help.

    Ok, I'll start looking into this and post updates here.

    @ericsnowcurrently
    Copy link
    Member Author

    On Tue, Oct 19, 2021 at 10:47 AM Marc-Andre Lemburg
    <report@bugs.python.org> wrote:

    Ok, I'll start looking into this and post updates here.

    Thanks!

    @malemburg
    Copy link
    Member

    On 19.10.2021 18:47, Marc-Andre Lemburg wrote:
    > 
    >> On Sat, Oct 16, 2021 at 5:01 AM Marc-Andre Lemburg
    >> <report@bugs.python.org> wrote:
    >>> I can try to port PyRun to 3.9 and 3.10 to see whether I run into any issues.
    >>> Would that help ?
    >>
    >> Yeah, that would totally help.
    > 
    > Ok, I'll start looking into this and post updates here.

    I have PyRun mostly working with Python 3.9. Still need to add a few
    new C modules, but the basics work.

    No changes were necessary to Tools/freeze/. The PGO build complains
    about test_embed not working - no surprise there. I'll patch the suite
    to ignore the test.

    BTW: Why is test_embed even used for the PGO target ?

    @ericsnowcurrently
    Copy link
    Member Author

    On Wed, Oct 20, 2021 at 6:01 AM Marc-Andre Lemburg
    <report@bugs.python.org> wrote:

    I have PyRun mostly working with Python 3.9.
    ...
    No changes were necessary to Tools/freeze/.

    Great! Thanks for getting to that so quickly. Are you going to take
    a look at 3.10 after you're happy with 3.9?

    BTW: Why is test_embed even used for the PGO target ?

    Perhaps I've missed something, but I'm not clear on why PGO would be a
    problem for test_embed. Are you talking about a specific test in
    test_embed?

    @malemburg
    Copy link
    Member

    On 20.10.2021 16:25, Eric Snow wrote:

    Eric Snow <ericsnowcurrently@gmail.com> added the comment:

    On Wed, Oct 20, 2021 at 6:01 AM Marc-Andre Lemburg
    <report@bugs.python.org> wrote:
    > I have PyRun mostly working with Python 3.9.
    > ...
    > No changes were necessary to Tools/freeze/.

    Great! Thanks for getting to that so quickly. Are you going to take
    a look at 3.10 after you're happy with 3.9?

    Yes, 3.10 is next, once I have 3.9 ironed out. And then I'll give
    3.11 a try.

    > BTW: Why is test_embed even used for the PGO target ?

    Perhaps I've missed something, but I'm not clear on why PGO would be a
    problem for test_embed. Are you talking about a specific test in
    test_embed?

    Sorry, I wasn't clear. PGO is not a problem for test_embed. I just
    wonder why the test_embed tests are run for creating the PGO profile
    files. test_embed is far from being a regular work load for Python
    applications.

    Well, I guess using the test suite for PGO is questionable anyway.
    It's just that we don't have anything else handy to create those
    profiles at Python build time :-)

    @ericsnowcurrently
    Copy link
    Member Author

    On Wed, Oct 20, 2021 at 8:38 AM Marc-Andre Lemburg
    <report@bugs.python.org> wrote:

    Yes, 3.10 is next, once I have 3.9 ironed out. And then I'll give 3.11 a try.

    Thanks!

    Sorry, I wasn't clear. PGO is not a problem for test_embed. I just
    wonder why the test_embed tests are run for creating the PGO profile
    files. test_embed is far from being a regular work load for Python
    applications.

    Well, I guess using the test suite for PGO is questionable anyway.
    It's just that we don't have anything else handy to create those
    profiles at Python build time :-)

    Ah, that makes sense. I expect we don't do anything more complex than
    run the whole suite. There have been discussions in the past about
    alternate workloads, but clearly nothing came of that. I suppose a
    comprehensive suite of high-level, (relatively) long-running
    benchmarks that cover the community's primary Python workloads would
    be ideal, but we don't have anything like that (yet). I suppose this
    is something we could discuss elsewhere. :)

    @malemburg
    Copy link
    Member

    I have an initial version of PyRun for Python 3.10 running as well.
    This created a few more headaches in order to make it work with
    setuptools and some glitches which appear to be bugs in 3.10
    (https://bugs.python.org/issue45563 and https://bugs.python.org/issue45562).
    Nothing major, though.

    I'll have to check my version of the freeze tool against the one
    in Python 3.9 and 3.10 to see whether there's anything in the
    core versions which could cause the tool not to work.

    BTW: (My) freeze.py uses this startup code as main():

    int
    main(int argc, char **argv)
    {
            extern int Py_FrozenMain(int, char **);
        /* Disabled, since we want to default to non-optimized mode: */
        /* Py_OptimizeFlag++; */
        Py_NoSiteFlag++;        /* Don't import site.py */
    
            PyImport_FrozenModules = _PyImport_FrozenModules;
            return Py_FrozenMain(argc, argv);
    }

    I still have to dig through the changes you have made, but this
    suggests that it replaces PyImport_FrozenModules completely
    with its own version, so the default freeze that you are
    implementing gets overridden.

    @ericsnowcurrently
    Copy link
    Member Author

    FYI, I figured out the problem on my end. I wasn't using an installed python. Once I did it worked fine.

    @malemburg
    Copy link
    Member

    On 26.10.2021 16:02, Eric Snow wrote:

    FYI, I figured out the problem on my end. I wasn't using an installed python. Once I did it worked fine.

    Oh, you mean you tried using it directly from the source tree ?
    I don't think I ever tried that direct route.

    When building PyRun, I first install to a temporary directory and
    then use this to run the freeze.py tool, generate the frozen .c
    files and run make to have the executable built.

    I've pretty much finished the port to 3.10.

    I'll try the main version in the next couple of days. There's currently
    a lot of work going on for the makesetup / Setup files
    (https://bugs.python.org/issue45548). I'm waiting for that to stabilize.

    @ericsnowcurrently
    Copy link
    Member Author

    New changeset 074fa57 by Eric Snow in branch 'main':
    bpo-45395: Make custom frozen modules additions instead of replacements. (gh-28778)
    074fa57

    @ericsnowcurrently
    Copy link
    Member Author

    This is done now.

    @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
    3.11 bug and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants