[issue17314] Stop using imp.find_module() in multiprocessing
Roundup Robot added the comment: 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 -- nosy: +python-dev ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17314 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17314] Stop using imp.find_module() in multiprocessing
Changes by Brett Cannon br...@python.org: -- resolution: - fixed stage: patch review - committed/rejected status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17314 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17314] Stop using imp.find_module() in multiprocessing
Brett Cannon added the comment: In the common case of SourceLoader it will set __loader__, __package__, __file__, and __cached__. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17314 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17314] Stop using imp.find_module() in multiprocessing
Brett Cannon added the comment:
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.pyFri May 24 13:51:21 2013 +0200
+++ b/Lib/multiprocessing/forking.pyFri 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
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17314
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17314] Stop using imp.find_module() in multiprocessing
Richard Oudkerk added the comment: Looks good to me. (Any particular reason for ignoring AttributeError?) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17314 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17314] Stop using imp.find_module() in multiprocessing
Brett Cannon added the comment: 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). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17314 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17314] Stop using imp.find_module() in multiprocessing
Richard Oudkerk added the comment: 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()? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17314 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17314] Stop using imp.find_module() in multiprocessing
New submission from Brett Cannon: 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?). -- assignee: sbt components: Library (Lib) files: remove_imp.find_module.diff keywords: patch messages: 183177 nosy: brett.cannon, sbt priority: normal severity: normal stage: patch review status: open title: Stop using imp.find_module() in multiprocessing versions: Python 3.4 Added file: http://bugs.python.org/file29270/remove_imp.find_module.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17314 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17314] Stop using imp.find_module() in multiprocessing
Richard Oudkerk added the comment:
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.)
--
Added file: http://bugs.python.org/file29274/mp-importlib.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17314
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17314] Stop using imp.find_module() in multiprocessing
Brett Cannon added the comment:
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.
--
assignee: sbt - brett.cannon
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17314
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
