bpo-33331: Clean modules in the reversed order in PyImport_Cleanup().#6565
Conversation
ncoghlan
left a comment
ncoghlan
left a comment
There was a problem hiding this comment.
Due to top level modules being added to sys.modules before the modules that they import, I think this will make things worse rather than better: https://bugs.python.org/issue33331#msg319135
I've added a couple of alternative suggestions for possible improvement to the tracker issue, though.
|
When you're done making the requested changes, leave the comment: And if you don't make the requested changes, you will be put in the comfy chair! |
Modules imported last are now cleared first at interpreter shutdown.
e4c87c3 to
db88b24
Compare
|
I have made the requested changes; please review again. Top level modules are now moved to the end of |
|
Thanks for making the requested changes! @ncoghlan, @brettcannon: please review the changes made to this pull request. |
| if spec.loader is None: | ||
| if spec.submodule_search_locations is None: | ||
| raise ImportError('missing loader', name=spec.name) | ||
| # namespace package |
There was a problem hiding this comment.
Namespace package.; capitalize and end in a period (and I realize this is a hold-over from how the code already was 😄 ).
| return sys.modules[spec.name] | ||
| # This must be done before putting the module in sys.modules | ||
| # (otherwise an optimization shortcut in import.c becomes | ||
| # wrong) |
|
@ncoghlan, does the PR look good to you now? |
ncoghlan
left a comment
ncoghlan
left a comment
There was a problem hiding this comment.
Sorry, I forgot I'd previously left a blocking review on this one. The move-to-the-end resolves the ordering concern I had, so this looks good to me now. Thanks!
5 participants
Modules imported last are now cleared first at interpreter shutdown.
https://bugs.python.org/issue33331