homepage

This issue tracker has been migrated to GitHub , and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Stop using imp.find_module() in multiprocessing
Type: Stage: resolved
Components: Library (Lib) Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: brett.cannon Nosy List: brett.cannon, python-dev, sbt
Priority: normal Keywords: patch

Created on 2013年02月27日 19:56 by brett.cannon, last changed 2022年04月11日 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
remove_imp.find_module.diff brett.cannon, 2013年02月27日 19:56 remove use of imp.find_module() from multiprocessing/forking.py review
mp-importlib.diff sbt, 2013年02月27日 23:20 review
Messages (9)
msg183177 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2013年02月27日 19:56
I'm trying to remove all uses of imp.find_module()/load_module() and multiprocessing seems to have a single use of both purely for (re)loading a module. The attached patch moves over to importlib.find_loader() and subsequent load_module() call to match the semantics of imp.find_module()/load_module(). If a guaranteed reload is not necessary then importlib.import_module() is a single-line change.
I ran the test suite, but there don't seem to be any explicit tests for this chunk of code (or am I missing something?).
msg183191 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2013年02月27日 23:20
I think this change will potentially make the main module get imported twice under different names when we transfer pickled data between processes. The current code (which is rather a mess) goes out of its way to avoid that.
Basically the main process makes sys.modules['__mp_main__'] an alias for the main module, and other process import the parent's main module with __name__ == '__mp_main__' and make sys.modules['__main__'] an alias for that. This means that any functions/classes defined in the main module (from whatever process) will have
 obj.__module__ in {'__main__', '__mp_main__'}
Unpickling such an object will succeed in any process without reimporting the main module.
Attached is an alternative patch which is more like the original code and seems to work. (Maybe modifying loader.name is an abuse of the API.)
msg183194 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2013年02月28日 01:03
It is an abuse since I didn't design that part of the API to function that way, but it's cool that it just happens to. =)
I do see your use-case and it is legitimate, although extremely rare and narrow. Let me think about whether I want to add specific support either through your approach, Richard, or if I want to decouple the setting of module attributes so that it is more along the lines of::
 main_module = imp.new_module('__mp_main__')
 loader.set_attributes(main_module) # BRAND-NEW; maybe private to the stdlib?
 main_module.__name__ = '__mp_main__'
 code_object = loader.get_code(main_name)
 sys.modules['__main__'] = sys.modules['__mp_main__'] = main_module # OLD
 exec(code_object, main_module.__dict__)
I'm currently leaning towards the latter option since it's an annoying bit to get right and it doesn't hurt anything to expose.
msg189967 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2013年05月25日 15:36
So I think I have come up with a way to expose a new method that makes this use-case doable and in a sane manner. Richard, let me know what you think so that I know that this makes sense before I commit myself to the new method (init_module_attrs())::
--- a/Lib/multiprocessing/forking.py	Fri May 24 13:51:21 2013 +0200
+++ b/Lib/multiprocessing/forking.py	Fri May 24 08:06:17 2013 -0400
@@ -449,7 +449,7 @@
 elif main_name != 'ipython':
 # Main modules not actually called __main__.py may
 # contain additional code that should still be executed
- import imp
+ import importlib
 
 if main_path is None:
 dirs = None
@@ -460,16 +460,17 @@
 
 assert main_name not in sys.modules, main_name
 sys.modules.pop('__mp_main__', None)
- file, path_name, etc = imp.find_module(main_name, dirs)
+ # We should not try to load __main__
+ # since that would execute 'if __name__ == "__main__"'
+ # clauses, potentially causing a psuedo fork bomb.
+ loader = importlib.find_loader(main_name, path=dirs)
+ main_module = imp.new_module(main_name)
 try:
- # We should not do 'imp.load_module("__main__", ...)'
- # since that would execute 'if __name__ == "__main__"'
- # clauses, potentially causing a psuedo fork bomb.
- main_module = imp.load_module(
- '__mp_main__', file, path_name, etc
- )
- finally:
- if file:
- file.close()
+ loader.init_module_attrs(main_module)
+ except AttributeError:
+ pass
+ main_module.__name__ = '__mp_main__'
+ code = loader.get_code(main_name)
+ exec(code, main_module.__dict__)
 
 sys.modules['__main__'] = sys.modules['__mp_main__'] = main_module
msg189991 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2013年05月25日 19:07
Looks good to me.
(Any particular reason for ignoring AttributeError?)
msg190031 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2013年05月25日 22:50
Catching the AttributeError is in case a loader is used that doesn't define init_module_attrs(). Since it will be new to Python 3.4 I can't guarantee that it will exist (in case someone doesn't subclass the proper ABC).
msg190036 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2013年05月25日 23:31
The unit tests pass with the patch already (if we don't delete the "import imp" line).
What attributes will be set by init_module_attrs()?
msg190089 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2013年05月26日 14:24
In the common case of SourceLoader it will set __loader__, __package__, __file__, and __cached__.
msg190758 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013年06月07日 15:45
New changeset 97adaa820353 by Brett Cannon in branch 'default':
Issue #17314: Stop using imp in multiprocessing.forking and move over
http://hg.python.org/cpython/rev/97adaa820353 
History
Date User Action Args
2022年04月11日 14:57:42adminsetgithub: 61516
2013年06月07日 15:46:09brett.cannonsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2013年06月07日 15:45:50python-devsetnosy: + python-dev
messages: + msg190758
2013年05月26日 14:24:05brett.cannonsetmessages: + msg190089
2013年05月25日 23:31:41sbtsetmessages: + msg190036
2013年05月25日 22:50:44brett.cannonsetmessages: + msg190031
2013年05月25日 19:07:05sbtsetmessages: + msg189991
2013年05月25日 15:36:10brett.cannonsetmessages: + msg189967
2013年02月28日 01:03:54brett.cannonsetassignee: sbt -> brett.cannon
messages: + msg183194
2013年02月27日 23:20:35sbtsetfiles: + mp-importlib.diff

messages: + msg183191
2013年02月27日 20:03:22brett.cannonlinkissue14797 dependencies
2013年02月27日 19:56:34brett.cannoncreate

AltStyle によって変換されたページ (->オリジナル) /