Skip to content

Clear more data in TypeChecker.reset() instead of asserting #19087

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

Merged
merged 1 commit into from
May 13, 2025

Conversation

svalentin
Copy link
Collaborator

@svalentin svalentin commented May 13, 2025

Running mypy daemon on internal Dropbox codebase can cause an AssertionError:

version: 1.16.0+dev.ca609acabdc94ee973a53d62b8dcb7e55c789aec
Daemon crashed!
Traceback (most recent call last):
  File "mypy/dmypy_server.py", line 237, in serve
  File "mypy/dmypy_server.py", line 286, in run_command
  File "mypy/dmypy_server.py", line 364, in cmd_check
  File "mypy/dmypy_server.py", line 428, in check
  File "mypy/dmypy_server.py", line 517, in initialize_fine_grained
  File "mypy/server/update.py", line 265, in update
  File "mypy/server/update.py", line 367, in update_one
  File "mypy/server/update.py", line 432, in update_module
  File "mypy/server/update.py", line 672, in update_module_isolated
  File "mypy/build.py", line 2410, in finish_passes
  File "mypy/build.py", line 2417, in free_state
  File "mypy/checker.py", line 443, in reset
AssertionError

Let's convert these asserts in reset() to actual cleanup. I see no reason not to clean them up.
It also seems safe for this particular crash, since in File "mypy/build.py", line 2417, in free_state
right after this line there's self._type_checker = None. So even if we wer not to call reset() everything would still be correct.

Alternatively, we can just reset everything by calling __init__ with original args:

self.__init__(self.errors, self.modules, self.options, self.tree, self.path, self.plugin, self.expr_checker.per_line_checking_time_ns)

Running mypy daemon on internal Dropbox codebase can cause an Assertion
Error:

```
version: 1.16.0+dev.ca609acabdc94ee973a53d62b8dcb7e55c789aec
Daemon crashed!
Traceback (most recent call last):
  File "mypy/dmypy_server.py", line 237, in serve
  File "mypy/dmypy_server.py", line 286, in run_command
  File "mypy/dmypy_server.py", line 364, in cmd_check
  File "mypy/dmypy_server.py", line 428, in check
  File "mypy/dmypy_server.py", line 517, in initialize_fine_grained
  File "mypy/server/update.py", line 265, in update
  File "mypy/server/update.py", line 367, in update_one
  File "mypy/server/update.py", line 432, in update_module
  File "mypy/server/update.py", line 672, in update_module_isolated
  File "mypy/build.py", line 2410, in finish_passes
  File "mypy/build.py", line 2417, in free_state
  File "mypy/checker.py", line 443, in reset
AssertionError
```

Let's convert these asserts in reset() to actual cleanup. I see no
reason not to clean them up.
It also seems safe for this particular crash, since in
File "mypy/build.py", line 2417, in free_state
right after this line there's `self._type_checker = None`. So even if we
wer not to call reset() everything would still be correct.
Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks reasonable at least as a temporary fix, but the assertion failure may be a symptom of another problem, so we should investigate this in more detail before the next release.

Copy link
Contributor

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

@svalentin svalentin merged commit 9e45dad into python:master May 13, 2025
18 checks passed
@svalentin svalentin deleted the dmypy-crash branch May 13, 2025 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants