Skip to content

Namespaced loader cleanup #58630

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
Oct 7, 2020
Merged

Namespaced loader cleanup #58630

merged 1 commit into from
Oct 7, 2020

Conversation

s0undt3ch
Copy link
Collaborator

What does this PR do?

See title

@s0undt3ch
Copy link
Collaborator Author

re-run full all

1 similar comment
@s0undt3ch
Copy link
Collaborator Author

re-run full all

@s0undt3ch
Copy link
Collaborator Author

re-run full all

1 similar comment
@s0undt3ch
Copy link
Collaborator Author

re-run full all

@s0undt3ch s0undt3ch changed the title [WIP] Revert loader namespacing Namespaced loader cleanup Oct 7, 2020
@s0undt3ch s0undt3ch requested a review from dwoz October 7, 2020 06:35
@s0undt3ch s0undt3ch marked this pull request as ready for review October 7, 2020 06:35
@s0undt3ch s0undt3ch requested a review from a team as a code owner October 7, 2020 06:35
@ghost ghost requested review from DmitryKuzmenko and removed request for a team October 7, 2020 06:35
@s0undt3ch s0undt3ch force-pushed the hotfix/memleak branch 2 times, most recently from 270cc7e to f717519 Compare October 7, 2020 06:42
@s0undt3ch s0undt3ch added the Magnesium Mg release after Na prior to Al label Oct 7, 2020
This is done in two stages, first, the module instance in `sys.modules` is set to `None`.
Then at a second stage, when creating a loader with the same namespace, it's actually removed from `sys.modules`.

The reason for this two stage cleanup procedure is because this function might get called during the GC collection cycle and trigger https://bugs.python.org/issue40327

We seem to specially trigger this during the CI test runs with `coverage.py` tracking
the code usage:

    Traceback (most recent call last):
      File "/urs/lib64/python3.6/site-packages/coverage/multiproc.py", line 37, in _bootstrap
        cov.start()
      File "/urs/lib64/python3.6/site-packages/coverage/control.py", line 527, in start
        self._init_for_start()
      File "/urs/lib64/python3.6/site-packages/coverage/control.py", line 455, in _init_for_start
        concurrency=concurrency,
      File "/urs/lib64/python3.6/site-packages/coverage/collector.py", line 111, in __init__
        self.origin = short_stack()
      File "/urs/lib64/python3.6/site-packages/coverage/debug.py", line 157, in short_stack
        stack = inspect.stack()[limit:skip:-1]
      File "/usr/lib64/python3.6/inspect.py", line 1501, in stack
        return getouterframes(sys._getframe(1), context)
      File "/usr/lib64/python3.6/inspect.py", line 1478, in getouterframes
        frameinfo = (frame,) + getframeinfo(frame, context)
      File "/usr/lib64/python3.6/inspect.py", line 1452, in getframeinfo
        lines, lnum = findsource(frame)
      File "/usr/lib64/python3.6/inspect.py", line 780, in findsource
        module = getmodule(object, file)
      File "/usr/lib64/python3.6/inspect.py", line 732, in getmodule
        for modname, module in list(sys.modules.items()):
    RuntimeError: dictionary changed size during iteration
@s0undt3ch
Copy link
Collaborator Author

re-run full all

@s0undt3ch
Copy link
Collaborator Author

The pytest failures are expected at this stage. Will be fixed afterwards. Merging.

@s0undt3ch s0undt3ch merged commit dec7a4a into master Oct 7, 2020
@s0undt3ch s0undt3ch deleted the hotfix/memleak branch October 7, 2020 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Magnesium Mg release after Na prior to Al
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants