Skip to content

Importing packages can poison the mypy cache. #5534

Closed
Closed
Importing packages can poison the mypy cache.#5534
Labels
crashpriority-0-high

Description

@TheKevJames
TheKevJames
opened on Aug 28, 2018

Ok -- this is a weird one, but I think I've got it down to the smallest possible repro.

I first noticed this issue with the aiohttp library as of version 3.4.0 (when they introduced type-hinting), but I'm cross-posting this issue to both projects since the issue seems to run deeper than just a library issue. Importing the aiohttp library, even if it is unused, causes the mypy cache to occasionally get poisoned.

Here's the repro case:

import aiohttp
def trigger() -> dict:
    return 'foo'

Running python -m mypy repro.py against this file works as expected (eg. returns an incompatible return type error). Replacing the -> dict with -> str throws the following assertion (attached at bottom of report) on re-running the type check. Note that this holds true in the opposite direction -- if we begin with -> str, the first typecheck is successful, but replacing -> str with -> dict causes the same assertion error. Either way, the situation can only be resolved by clearing the .mypy_cache folder, at which point mypy acts as expected once more.

Interestingly, removing the aiohttp import prevents this poisoning from occurring. In addition, removing the import once the cache is poisoned also fixes the issue.

I speculate there are two issues here, one with aiohttp and one with mypy:

  • aiohttp seems to have some weirdness going on in their type hinting
  • regardless of what a library does, the above repro case should not cause mypy issues when dealing only with stdlib types.

Error Trace:

Traceback (most recent call last):
  File "/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/lib/python3.7/runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/lib/python3.7/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/Users/kevin/coding/mypy-bug/venv/lib/python3.7/site-packages/mypy/__main__.py", line 11, in <module>
    main(None)
  File "/Users/kevin/coding/mypy-bug/venv/lib/python3.7/site-packages/mypy/main.py", line 91, in main
    res = type_check_only(sources, bin_dir, options, flush_errors, fscache)  # noqa
  File "/Users/kevin/coding/mypy-bug/venv/lib/python3.7/site-packages/mypy/main.py", line 148, in type_check_only
    fscache=fscache)
  File "/Users/kevin/coding/mypy-bug/venv/lib/python3.7/site-packages/mypy/build.py", line 177, in build
    flush_errors, fscache)
  File "/Users/kevin/coding/mypy-bug/venv/lib/python3.7/site-packages/mypy/build.py", line 350, in _build
    graph = dispatch(sources, manager)
  File "/Users/kevin/coding/mypy-bug/venv/lib/python3.7/site-packages/mypy/build.py", line 2560, in dispatch
    process_graph(graph, manager)
  File "/Users/kevin/coding/mypy-bug/venv/lib/python3.7/site-packages/mypy/build.py", line 2846, in process_graph
    process_fresh_modules(graph, prev_scc, manager)
  File "/Users/kevin/coding/mypy-bug/venv/lib/python3.7/site-packages/mypy/build.py", line 2932, in process_fresh_modules
    graph[id].fix_cross_refs()
  File "/Users/kevin/coding/mypy-bug/venv/lib/python3.7/site-packages/mypy/build.py", line 1975, in fix_cross_refs
    self.options.use_fine_grained_cache)
  File "/Users/kevin/coding/mypy-bug/venv/lib/python3.7/site-packages/mypy/fixup.py", line 24, in fixup_module
    node_fixer.visit_symbol_table(tree.names)
  File "/Users/kevin/coding/mypy-bug/venv/lib/python3.7/site-packages/mypy/fixup.py", line 89, in visit_symbol_table
    value.node.accept(self)
  File "/Users/kevin/coding/mypy-bug/venv/lib/python3.7/site-packages/mypy/nodes.py", line 2539, in accept
    return visitor.visit_type_alias(self)
  File "/Users/kevin/coding/mypy-bug/venv/lib/python3.7/site-packages/mypy/fixup.py", line 135, in visit_type_alias
    a.target.accept(self.type_fixer)
  File "/Users/kevin/coding/mypy-bug/venv/lib/python3.7/site-packages/mypy/types.py", line 511, in accept
    return visitor.visit_instance(self)
  File "/Users/kevin/coding/mypy-bug/venv/lib/python3.7/site-packages/mypy/fixup.py", line 149, in visit_instance
    inst.type = lookup_qualified_typeinfo(self.modules, type_ref, self.quick_and_dirty)
  File "/Users/kevin/coding/mypy-bug/venv/lib/python3.7/site-packages/mypy/fixup.py", line 239, in lookup_qualified_typeinfo
    assert quick_and_dirty, "Should never get here in normal mode"
AssertionError: Should never get here in normal mode

Versions:

(venv) kevin@box ~/c/mypy-bug » python --version
Python 3.7.0
(venv) kevin@box ~/c/mypy-bug » python -m mypy --version
mypy 0.620
(venv) kevin@box ~/c/mypy-bug » python -m pip freeze | grep aiohttp
aiohttp==3.4.0

Activity

  1. gvanrossum commented on Aug 28, 2018

    @gvanrossum
    gvanrossum
    on Aug 28, 2018
    Member

    What do you get without --ignore-missing-imports?

  2. TheKevJames commented on Aug 28, 2018

    @TheKevJames
    TheKevJames
    on Aug 28, 2018
    Author

    @gvanrossum I have verified the behavior is the same without that flag. I will edit my repro case to remove the flag, since it is unrelated.

  3. gvanrossum commented on Aug 28, 2018

    @gvanrossum
    gvanrossum
    on Aug 28, 2018
    Member

    OK, so then it would be interesting to see which part of aiohttp 3.4.0 causes the problem. That will be a difficult bisection job (remove code from aiohttp until the problem no longer occurs).

    But first, it's been over a month since mypy 0.620 was released, and the bugfix elves have been hard at work all that time. Does the problem still occur with mypy master from GitHub?

  4. TheKevJames commented on Aug 28, 2018

    @TheKevJames
    TheKevJames
    on Aug 28, 2018
    Author

    As of the current latest on master (f0b26f9), this is indeed still reproducible. For the record, the verified reproduction was after running:

    (venv) » pip install -U git+git://github.com/python/mypy.git
Collecting git+git://github.com/python/mypy.git
  Cloning git://github.com/python/mypy.git to /private/var/folders/qs/b8cnmbcd1szb8fv4j56_6p_w0000gn/T/pip-req-build-pf17uyhk
  Installing build dependencies ... done
Requirement already satisfied, skipping upgrade: typed-ast<1.2.0,>=1.1.0 in ./venv/lib/python3.7/site-packages (from mypy==0.630+dev.f0b26f9e6197eeb2f755fae73c949ff01384ded3) (1.1.0)
Collecting mypy_extensions<0.5.0,>=0.4.0 (from mypy==0.630+dev.f0b26f9e6197eeb2f755fae73c949ff01384ded3)
  Downloading https://files.pythonhosted.org/packages/4d/72/8d54e2b296631b9b14961d583e56e90d9d7fba8a240d5ce7f1113cc5e887/mypy_extensions-0.4.1-py2.py3-none-any.whl
Installing collected packages: mypy-extensions, mypy
  Found existing installation: mypy 0.620
    Uninstalling mypy-0.620:
      Successfully uninstalled mypy-0.620
  Running setup.py install for mypy ... done
Successfully installed mypy-0.630+dev.f0b26f9e6197eeb2f755fae73c949ff01384ded3 mypy-extensions-0.4.1

    in the original venv.

  5. gvanrossum commented on Aug 28, 2018

    @gvanrossum
    gvanrossum
    on Aug 28, 2018
    Member

    Thanks for confirming -- then bisecting by removing large parts of aiohttp is in order...

  6. ilevkivskyi commented on Aug 28, 2018

    @ilevkivskyi
    ilevkivskyi
    on Aug 28, 2018
    Member

    There is a hint in the traceback: the problematic type is aliased somewhere at a module level. This should narrow down the search area.

  7. ilevkivskyi
    added
    crash
    priority-0-high
    on Aug 29, 2018

  8. ilevkivskyi commented on Aug 29, 2018

    @ilevkivskyi
    ilevkivskyi
    on Aug 29, 2018
    Member

    Here is a minimalist repro:

    # file main.py
import lib
x = 1

# file lib.py
class Slow:
    pass

s: Slow  # could be anything that uses 'Slow' as a type: function, type alias, etc.
from cext import Slow  # type: ignore

    Run mypy main.py once, make any change to main.py, run again and see the crash.

    And this is what is going on: first mypy creates an Instance of Slow (TypeInfo at this time) as a type for s, but then the ignored import actually overwrites Slow in the symbol table making it a Var. Then when loading the cache mypy panics when it sees an Instance whose .type is a Var.

    The best solution IMO is that import over an existing name should never shadow/overwrite an existing node in the symbol table. The original node should be the "wining" one.

    Later we can support conditional imports by creating a "union" of two nodes, but we have a separate issue for this.

  9. ilevkivskyi commented on Aug 29, 2018

    @ilevkivskyi
    ilevkivskyi
    on Aug 29, 2018
    Member

    Another thing pointed out by @asvetlov is that the assert message could be a bit more informative. I will improve it as a part of the PR (just showing a full name and type of problematic node would be much better).

  10. asvetlov commented on Aug 29, 2018

    @asvetlov
    asvetlov
    on Aug 29, 2018

    Interesting. Thanks.
    The minimalist repro is exactly the aiohttp case.
    We use it for optional C Extention optimizations.
    CPython itself does the same but without type annotations.

  11. ilevkivskyi
    closed this as completedin #5565on Sep 4, 2018
  12. ilevkivskyi
    added a commit that references this issue on Sep 4, 2018

    Original node wins in case of a redefinition (especially if it is a c…

    cd6712d

  13. asvetlov commented on Sep 4, 2018

    @asvetlov
    asvetlov
    on Sep 4, 2018

    thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    crashpriority-0-high

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions