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

Unexpected exception with zip importer #89346

Closed
ronaldoussoren opened this issue Sep 13, 2021 · 8 comments
Closed

Unexpected exception with zip importer #89346

ronaldoussoren opened this issue Sep 13, 2021 · 8 comments
Assignees
Labels
3.10 only security fixes 3.11 only security fixes type-bug An unexpected behavior, bug, or error

Comments

@ronaldoussoren
Copy link
Contributor

BPO 45183
Nosy @Yhg1s, @brettcannon, @ronaldoussoren, @pablogsal, @miss-islington
PRs
  • bpo-45183: don't raise an exception when calling zipimport.zipimporter.find_spec() when the zip file is missing and the internal cache has been reset #28435
  • [3.10] bpo-45183: don't raise an exception when calling zipimport.zipimporter.find_spec() when the zip file is missing and the internal cache has been reset (GH-28435) #28437
  • [3.10] bpo-45183: don't raise an exception when calling zipimport.zipimporter.find_spec() when the zip file is missing and the internal cache has been reset (GH-28435) #28438
  • Files
  • repro.py
  • 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/brettcannon'
    closed_at = <Date 2021-09-18.00:47:38.847>
    created_at = <Date 2021-09-13.11:29:05.001>
    labels = ['type-bug', '3.10', '3.11']
    title = 'Unexpected exception with zip importer'
    updated_at = <Date 2021-09-18.00:47:38.846>
    user = 'https://github.com/ronaldoussoren'

    bugs.python.org fields:

    activity = <Date 2021-09-18.00:47:38.846>
    actor = 'brett.cannon'
    assignee = 'brett.cannon'
    closed = True
    closed_date = <Date 2021-09-18.00:47:38.847>
    closer = 'brett.cannon'
    components = []
    creation = <Date 2021-09-13.11:29:05.001>
    creator = 'ronaldoussoren'
    dependencies = []
    files = ['50279']
    hgrepos = []
    issue_num = 45183
    keywords = ['patch', '3.10regression']
    message_count = 8.0
    messages = ['401698', '401793', '401794', '401935', '402109', '402111', '402114', '402115']
    nosy_count = 5.0
    nosy_names = ['twouters', 'brett.cannon', 'ronaldoussoren', 'pablogsal', 'miss-islington']
    pr_nums = ['28435', '28437', '28438']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue45183'
    versions = ['Python 3.10', 'Python 3.11']

    @ronaldoussoren
    Copy link
    Contributor Author

    The attached file demonstrates the problem:

    If importlib.invalidate_caches() is called while the zipfile used by the zip importer is not available the import system breaks entirely. I found this in a testsuite that accedently did this (it should have updated sys.path).

    I get the following exception:

    $ python3.10 t.py
    Traceback (most recent call last):
      File "/Users/ronald/Projects/modulegraph2/t.py", line 27, in <module>
        import uu
      File "<frozen importlib._bootstrap>", line 1027, in _find_and_load
      File "<frozen importlib._bootstrap>", line 1002, in _find_and_load_unlocked
      File "<frozen importlib._bootstrap>", line 945, in _find_spec
      File "<frozen importlib._bootstrap_external>", line 1430, in find_spec
      File "<frozen importlib._bootstrap_external>", line 1402, in _get_spec
      File "<frozen zipimport>", line 168, in find_spec
      File "<frozen zipimport>", line 375, in _get_module_info
    TypeError: argument of type 'NoneType' is not iterable

    This exception is not very friendly....

    This particular exception is caused by setting self._files to None in the importer's invalidate_caches method, while not checking for None in _get_modules_info.

    I'm not sure what the best fix would be, setting self._files to an empty list would likely be the easiest fix.

    Note that the script runs without errors in Python 3.9.

    @ronaldoussoren ronaldoussoren added 3.10 only security fixes 3.11 only security fixes type-bug An unexpected behavior, bug, or error labels Sep 13, 2021
    @pablogsal
    Copy link
    Member

    I bisected this to:

    3abf6f0 is the first bad commit
    commit 3abf6f0
    Author: Desmond Cheong <desmondcheongzx@gmail.com>
    Date: Tue Mar 9 04:06:02 2021 +0800

    bpo-14678: Update zipimport to support importlib.invalidate_caches() (GH-24159)
    
    
    
    Added an invalidate_caches() method to the zipimport.zipimporter class based on the implementation of importlib.FileFinder.invalidate_caches(). This was done by adding a get_files() method and an _archive_mtime attribute to zipimport.zipimporter to check for updates or cache invalidation whenever the cache of files and toc entry information in the zipimporter is accessed.
    

    Doc/library/zipimport.rst | 9 +
    Lib/test/test_zipimport.py | 41 +
    Lib/zipimport.py | 10 +
    .../2021-01-07-21-25-49.bpo-14678.1zniCH.rst | 3 +
    Python/importlib_zipimport.h | 1896 ++++++++++----------
    5 files changed, 1020 insertions(+), 939 deletions(-)
    create mode 100644 Misc/NEWS.d/next/Library/2021-01-07-21-25-49.bpo-14678.1zniCH.rst
    bisect run success

    Which is https://bugs.python.org/issue14678

    @pablogsal
    Copy link
    Member

    Brett, can you take a look when you have some time?

    @ronaldoussoren
    Copy link
    Contributor Author

    I just noticed that I'm unnecessarily obtuse in my description of a possible fix, the diff (without test update):

    % git diff Lib/zipimport.py                                                                                        (main)cpython
    diff --git a/Lib/zipimport.py b/Lib/zipimport.py
    index c55fec6aa1..43ac6cbe57 100644
    --- a/Lib/zipimport.py
    +++ b/Lib/zipimport.py
    @@ -334,7 +334,7 @@ def invalidate_caches(self):
                 _zip_directory_cache[self.archive] = self._files
             except ZipImportError:
                 _zip_directory_cache.pop(self.archive, None)
    -            self._files = None
    +            self._files = {}
     
     
         def __repr__(self):

    With that change the exception should not happen, and the now stale zipimporter would be ignored when flushing the cache while the zipfile referenced by the zip importer instance has been removed.

    That said, I haven't tested this and won't create a PR because my local tree is (still) a mess.

    @brettcannon
    Copy link
    Member

    The proposed fix seems to be the right one based on my reading of the code.

    @brettcannon
    Copy link
    Member

    New changeset 209b703 by Brett Cannon in branch 'main':
    bpo-45183: don't raise an exception when calling zipimport.zipimporter.find_spec() when the zip file is missing and the internal cache has been reset (GH-28435)
    209b703

    @brettcannon
    Copy link
    Member

    New changeset e1bdecb by Brett Cannon in branch '3.10':
    [3.10] bpo-45183: don't raise an exception when calling zipimport.zipimporter.find_spec() when the zip file is missing and the internal cache has been reset (GH-28435) (bpo-28438)
    e1bdecb

    @brettcannon
    Copy link
    Member

    I decided that find_spec() saying something wasn't available in a finder made sense even though it was because a zip file no longer existed as the loader would still fail as appropriate.

    @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.10 only security fixes 3.11 only security fixes type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants