classification
Title: sys.modules cannot be reassigned
Type: Stage: needs patch
Components: Documentation Versions: Python 3.4, Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: brett.cannon Nosy List: Valentin.Lorentz, Yogesh.Chaudhari, brett.cannon, eric.snow, pitrou, python-dev, r.david.murray, terry.reedy
Priority: normal Keywords: 3.3regression, easy, patch

Created on 2013-05-11 08:23 by Valentin.Lorentz, last changed 2013-06-25 05:14 by eric.snow. This issue is now closed.

Files
File name Uploaded Description Edit
issue17953.patch Yogesh.Chaudhari, 2013-05-18 16:06 Adds statement "However, replacing it or deleting critical entries may cause Python to fail." review
Messages (12)
msg188901 - (view) Author: ProgVal (Valentin.Lorentz) Date: 2013-05-11 08:23
In Python 3.3 (I did not test with 3.4), sys.modules cannot be reassigned without breaking the import mechanism; while it works with Python <= 3.2.

Here is how to reproduce the bug:

progval@Andromede:/tmp$ mkdir test_imports
progval@Andromede:/tmp$ echo "from . import module" > test_imports/__init__.py
progval@Andromede:/tmp$ echo "print('foo')" > test_imports/module.py
progval@Andromede:/tmp$ python3.3
Python 3.3.1 (default, Apr  6 2013, 13:58:40) 
[GCC 4.7.2] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sys.modules = dict(sys.modules)
>>> import test_imports
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "./test_imports/__init__.py", line 1, in <module>
    from . import module
SystemError: Parent module 'test_imports' not loaded, cannot perform relative import
>>> 
progval@Andromede:/tmp$ python3.2
Python 3.2.3 (default, May  6 2013, 01:46:35) 
[GCC 4.7.2] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sys.modules = dict(sys.modules)
>>> import test_imports
foo
>>>
msg188914 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-05-11 12:34
I wouldn't call it a bug personally. The modules dictionary is used in all kinds of places in the interpreter; you can change the dictionary's contents, but not swap it with another one.

It's just a pity that we can't forbid reassignment altogether.
msg188945 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2013-05-11 18:53
There is no good way to solve this. At the C level there interpreter struct has two key fields, sysdict and modules. The former is sys.__dict__ and the latter is sys.modules. But when you re-assign sys.modules you then break the assumption that sys.modules is the same dict as that contained in interp->modules. And this all goes out the window as the C code is expected to use interp->modules while the Python code in importlib only has access to sys.modules. The reason this used to "work" is your new dictionary was completely ignored and so we basically a no-op from the perspective of import (done in Python 2.7 but same result in any version up to Python 3.3)::

  >>> import sys
  >>> original_modules = sys.modules
  >>> new_modules = sys.modules.copy()
  >>> sys.modules = new_modules
  >>> import pkg
  >>> 'pkg' in original_modules
  True
  >>> 'pkg' in new_modules
  False

What really needs to happen is that sys.modules needs to be documented as something that cannot be replaced. If you really want to update it cleanly then do ``sys.modules.clear(); sys.modules.update(new_modules)``, but even that is tricky because removing certain modules will flat-out break Python.

I have updated the issue to be a documentation one and added Python 3.4 to the affected versions.
msg189312 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013-05-15 21:51
"sys.modules
    This is a dictionary that maps module names to modules which have already been loaded. This can be manipulated to force reloading of modules and other tricks."

How about adding at the end
"However, replacing or clearing it may cause Python to fail."

or to be slightly more explicit "However, replacing it or deleting critical entries may cause Python to fail."
msg189317 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2013-05-15 22:30
Sounds good to me.
msg189529 - (view) Author: Yogesh Chaudhari (Yogesh.Chaudhari) * Date: 2013-05-18 16:06
Documentation added for sys.modules
msg189933 - (view) Author: Roundup Robot (python-dev) Date: 2013-05-24 21:31
New changeset 4f8160e45cb7 by Brett Cannon in branch '3.3':
Issue #17953: document that sys.modules shouldn't be replaced (thanks
http://hg.python.org/cpython/rev/4f8160e45cb7

New changeset b60ae4ebf981 by Brett Cannon in branch 'default':
merge fix for issue #17953
http://hg.python.org/cpython/rev/b60ae4ebf981
msg189934 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2013-05-24 21:33
Thanks for the patch, Yogesh, but I went with different wording that I came up with on the train the other day while offline. But you did remind me to update Misc/NEWS (which led to a discussion on python-committers), so thanks for that.
msg189938 - (view) Author: Yogesh Chaudhari (Yogesh.Chaudhari) * Date: 2013-05-25 01:13
You are most welcome Brett. I am just starting to contribute with this doc fix. Didn't think a small detail in it will generate such a long debate in the python-committers mailing list. 

Is there a plan in action for 'fixing' Misc/NEWS. I now realize that this change is too small to warrant an entry there, however, as someone new to the list, I simply followed http://docs.python.org/devguide/patch.html. Doing a make patchcheck warns about no changes to Misc/NEWS. 

IMHO it would be great if this tool can look at the files changed (or maybe at-least just the directory, or even, simply number of lines modified) and decide if an entry is required in Misc/News. Of-course this is from my perspective. I would like to get your view on this and help about the solution if possible.
msg189939 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013-05-25 03:07
I think that is a reasonable suggestion for an enhancement to patchcheck (though I think Brett had a good reason to add a news entry in this particular case).  You could open a new issue with that suggestion.  On the other hand, patchcheck always warns about other stuff that it can't so easily realize aren't really needed, so it may not be considered a worthwhile enhancement.
msg189940 - (view) Author: Yogesh Chaudhari (Yogesh.Chaudhari) * Date: 2013-05-25 03:32
That sounds good. I will open up an issue (if not for anything else other than to get more eyes on this issue)
msg191833 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2013-06-25 05:14
For the record, issue12633 has some more discussion on this.
History
Date User Action Args
2013-06-25 05:14:26eric.snowsetnosy: + eric.snow
messages: + msg191833
2013-05-25 03:32:18Yogesh.Chaudharisetmessages: + msg189940
2013-05-25 03:07:55r.david.murraysetnosy: + r.david.murray
messages: + msg189939
2013-05-25 01:13:08Yogesh.Chaudharisetmessages: + msg189938
2013-05-24 21:33:08brett.cannonsetstatus: open -> closed
resolution: fixed
messages: + msg189934
2013-05-24 21:31:46python-devsetnosy: + python-dev
messages: + msg189933
2013-05-18 16:06:35Yogesh.Chaudharisetfiles: + issue17953.patch

nosy: + Yogesh.Chaudhari
messages: + msg189529

keywords: + patch
2013-05-15 22:30:57brett.cannonsetmessages: + msg189317
2013-05-15 21:51:59terry.reedysetnosy: + terry.reedy

messages: + msg189312
stage: test needed -> needs patch
2013-05-11 18:53:41brett.cannonsetversions: + Python 3.4
messages: + msg188945

components: + Documentation, - Interpreter Core
keywords: + easy
type: behavior ->
stage: test needed
2013-05-11 12:34:34pitrousetnosy: + pitrou
messages: + msg188914
2013-05-11 12:32:05christian.heimessetkeywords: + 3.3regression
assignee: brett.cannon

nosy: + brett.cannon
2013-05-11 08:23:33Valentin.Lorentzcreate