[issue19946] Handle a non-importable __main__ in multiprocessing

[Nick Coghlan]

Nick Coghlan added the comment:

Now passing on all the stable buildbots (the two red Windows bots are for other 
issues, such as issue 15599 for the threaded import test failure)

--
status: pending -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19946] Handle a non-importable __main__ in multiprocessing

[Nick Coghlan]

Nick Coghlan added the comment:

Pending a clean bill of health from the stable buildbots :)

--
status: open -> pending

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19946] Handle a non-importable __main__ in multiprocessing

[Roundup Robot]

Roundup Robot added the comment:

New changeset 00d09afb57ca by Nick Coghlan in branch 'default':
Issue #19946: use public API for multiprocessing start methods
http://hg.python.org/cpython/rev/00d09afb57ca

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19946] Handle a non-importable __main__ in multiprocessing

[Nick Coghlan]

Nick Coghlan added the comment:

Ah, I should have looked more closely at the docs to see if there was a public 
API for that before poking around in the package internals.

In that case, I suggest we change this bit in the test:

# We look inside the context module to find out which
# start methods we can check
from multiprocessing.context import _concrete_contexts

to use the appropriate public API:

# Need to know which start methods we should test
import multiprocessing
AVAILABLE_START_METHODS = set(multiprocessing.get_all_start_methods())

And then adjust the skip check to look in AVAILABLE_START_METHODS rather than 
_concrete_contexts.

I'll make that change tonight if nobody beats me to it.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19946] Handle a non-importable __main__ in multiprocessing

[Richard Oudkerk]

Richard Oudkerk added the comment:

On 19/12/2013 10:00 pm, Nick Coghlan wrote:
> I think that needs to be fixed on the multiprocessing side rather than just
> in the tests - we shouldn't create a concrete context for a start method
> that isn't going to work on that platform. Finding that kind of discrepancy
> was part of my rationale for basing the skips on the available contexts
> (although my main motivation was simplicity).
>
> There may also be docs implications in describing which methods are
> supported on different platforms (although I haven't looked at how that is
> currently documented).
>
If by "concrete context" you mean _concrete_contexts['forkserver'], then 
that is supposed to be private.  If you write

 ctx = multiprocessing.get_context('forkserver')

then this will raise ValueError if the forkserver method is not 
available.  You can also use

'forkserver' in multiprocessing.get_all_start_methods()

to check if it is available.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19946] Handle a non-importable __main__ in multiprocessing

[Nick Coghlan]

Nick Coghlan added the comment:

I think that needs to be fixed on the multiprocessing side rather than just
in the tests - we shouldn't create a concrete context for a start method
that isn't going to work on that platform. Finding that kind of discrepancy
was part of my rationale for basing the skips on the available contexts
(although my main motivation was simplicity).

There may also be docs implications in describing which methods are
supported on different platforms (although I haven't looked at how that is
currently documented).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19946] Handle a non-importable __main__ in multiprocessing

[Christian Heimes]

Christian Heimes added the comment:

The OpenIndiana tests are still failing. OpenIndiana doesn't support forkserver 
because it doesn't implement the send handle feature. The patch skips the 
forkserver tests if HAVE_SEND_HANDLE is false.

--
Added file: http://bugs.python.org/file33222/skip_forkserver.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19946] Handle a non-importable __main__ in multiprocessing

[Roundup Robot]

Roundup Robot added the comment:

New changeset 460961e80e31 by Nick Coghlan in branch 'default':
Issue #19946: appropriately skip new multiprocessing tests
http://hg.python.org/cpython/rev/460961e80e31

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19946] Handle a non-importable __main__ in multiprocessing

[Zachary Ware]

Zachary Ware added the comment:

The problem on Windows at least is that the skips for the 'fork' and 
'forkserver' start methods aren't firing due to setUpClass being improperly set 
up in MultiProcessingCmdLineMixin: it's not decorated as a classmethod and the 
'u' is lower-case instead of upper.  Just fixing that makes for some unusual 
output ("skipped '"fork" start method not available'" with no indication of 
which test was skipped) and a variable number of tests depending on available 
start methods, so a better fix is to just do the check in setUp.

Unrelated to the failure, but we're also in the process of moving away from 
using test_main(), preferring unittest.main().

The attached patch addresses both, passes on Windows and Linux, and I suspect 
should help on OpenIndiana as well judging by the tracebacks it's giving.

--
nosy: +zach.ware
Added file: http://bugs.python.org/file33201/issue19946.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19946] Handle a non-importable __main__ in multiprocessing

[Christian Heimes]

Christian Heimes added the comment:

The commit broken a couple of buildbots like all Windows bots and OpenIndiana.

--
nosy: +christian.heimes
resolution: fixed -> 
status: closed -> open

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19946] Handle a non-importable __main__ in multiprocessing

[Richard Oudkerk]

Richard Oudkerk added the comment:

Thanks for your hard work Nick!

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19946] Handle a non-importable __main__ in multiprocessing

[Nick Coghlan]

Changes by Nick Coghlan :


Removed file: 
http://bugs.python.org/file33163/test_multiprocessing_main_handling.py

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19946] Handle a non-importable __main__ in multiprocessing

[Roundup Robot]

Roundup Robot added the comment:

New changeset b6d6f3b4b100 by Nick Coghlan in branch 'default':
Close #19946: use runpy as needed in multiprocessing
http://hg.python.org/cpython/rev/b6d6f3b4b100

--
resolution:  -> fixed
stage: patch review -> committed/rejected
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19946] Handle a non-importable __main__ in multiprocessing

[Nick Coghlan]

Changes by Nick Coghlan :


Removed file: 
http://bugs.python.org/file33146/issue19946_pep_451_multiprocessing.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19946] Handle a non-importable __main__ in multiprocessing

[Nick Coghlan]

Changes by Nick Coghlan :


Removed file: 
http://bugs.python.org/file33161/test_multiprocessing_main_handling.py

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19946] Handle a non-importable __main__ in multiprocessing

[Nick Coghlan]

Nick Coghlan added the comment:

OK, fixed test case attached. Turns out the ipython workaround test was 
completely wrong and never even loaded multiprocessing, and hence always 
passed, even with the workaround disabled. So I fixed that test case, and used 
the same approach for the zipfile, directory and package tests. I also fixed 
the submodule test to check that explicit relative imports work properly from 
__mp_main__ in the child processes.

With this updated test cast, the v2 patch handles everything correctly, but 
there are 4 failures on Linux without the patch. Specifically:

- test_basic_script_no_suffix fails for the spawn and forkserver start methods 
(the child processes fail to find a spec for __mp_main__)
- test_module_in_package fails for the spawn and forkserver start methods (the 
explicit relative import from __mp_main__ fails because the import system isn't 
initialised correctly in the child processes)

The former case is the one Olivier reported in this issue. It's a new case for 
3.4, since the spawn start method was previously only available on Windows, 
where scripts always have an extension.

The latter edge case is the one my "XXX (ncoghlan): The following code makes 
several bogus assumptions regarding the relationship between __file__ and a 
module's real name." comment was about.

I believe we could actually adjust earlier versions to handle things as well as 
this new PEP 451 based approach (by using a combination of __package__ and 
__file__ rather than __spec__), but that's much harder for me to work on in 
those versions where the "spawn" start method is only available on Windows :)

--
Added file: 
http://bugs.python.org/file33175/test_multiprocessing_main_handling.py

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19946] Handle a non-importable __main__ in multiprocessing

[Olivier Grisel]

Olivier Grisel added the comment:

I applied issue19946_pep_451_multiprocessing_v2.diff and I confirm that it 
fixes the problem that I reported initially.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19946] Handle a non-importable __main__ in multiprocessing

[Nick Coghlan]

Changes by Nick Coghlan :


--
stage: needs patch -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19946] Handle a non-importable __main__ in multiprocessing

[Nick Coghlan]

Nick Coghlan added the comment:

Updated test that handles timeouts better.

I also realised the current test failures are due to an error in the test 
design - the "failing" cases are ones where we deliberately *don't* rerun 
__main__ because the entire __main__.py file is assumed to be inside an 
implicit __main__-only guard.

So the code changes should be complete, I just need to figure out a way to 
tweak the tests appropriately.

--
Added file: 
http://bugs.python.org/file33163/test_multiprocessing_main_handling.py

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19946] Handle a non-importable __main__ in multiprocessing

[Nick Coghlan]

Nick Coghlan added the comment:

Current work in progress patch. The existing multiprocessing tests all pass, 
but the new main handling tests fail.

The fork start_method passes all the tests
The forkserver and spawn start methods fail the directory, zipfile and package 
tests.

--
Added file: 
http://bugs.python.org/file33162/issue19946_pep_451_multiprocessing_v2.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19946] Handle a non-importable __main__ in multiprocessing

[Nick Coghlan]

Nick Coghlan added the comment:

I created a test suite to ensure that all the various cases were handled 
correctly by the eventual patch (it doesn't test some of the namespace package 
related edge cases, but they devolve to normal module execution in terms of the 
final state of __main__, and that's covered by these tests).

--
Added file: 
http://bugs.python.org/file33161/test_multiprocessing_main_handling.py

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19946] Handle a non-importable __main__ in multiprocessing

[Nick Coghlan]

Nick Coghlan added the comment:

Scripts (whether in source form or precompiled) work via direct execution,
but all the other execution paths (directories, zipfiles, -m) rely on the
import system (via runpy). multiprocessing has been broken for years in
that regard, hence my old comment about the way it derived the module name
from the file name being problematic (although it only outright *broke*
with submodule execution, and even then you would likely get away with it
if you didn't use relative imports).

Historically, it was a hard problem to solve, since even the parent process
forgot the original name of __main__, but PEP 451 has now fixed that
limitation.

I also have an idea as to what may be wrong with my patch - I'm going to
try adjusting the first early return from _fixup_main_from_name to ensure
that __main__.__spec__ is set correctly.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19946] Handle a non-importable __main__ in multiprocessing

[Richard Oudkerk]

Richard Oudkerk added the comment:

> I appear to be somehow getting child processes where __main__.__file__ is
> set, but __main__.__spec__ is not.

That seems to be true for the __main__ module even when multiprocessing is not 
involved.  Running a file /tmp/foo.py containing

import sys
print(sys.modules['__main__'].__spec__, sys.modules['__main__'].__file__)

I get output

None /tmp/foo.py

I am confused by why you would ever want to load by module name rather than 
file name.  What problem would that fix?  If the idea is just to support 
importing a main module without a .py extension, isn't __file__ good enough?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19946] Handle a non-importable __main__ in multiprocessing

[Nick Coghlan]

Nick Coghlan added the comment:

With the restructuring in my patch, it would be easy enough to move the "early 
return" cases from the _fixup_main_* functions to instead be "don't set the 
variable" cases in get_preparation_data.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19946] Handle a non-importable __main__ in multiprocessing

[Nick Coghlan]

Nick Coghlan added the comment:

Bumping the priority on this, as multiprocessing is currently creating invalid 
child processes by failing to set __main__.__spec__ appropriately.

The attached patch is designed to get us started down that path. It's currently 
broken, but I need feedback from folks that know the multiprocessing code 
better than I do in order to know where best to start poking and prodding.

With the patch, invoking regrtest directly still works:

./python Lib/test/regrtest.py -v test_multiprocessing_spawn

But relying on module execution fails:
./python -m test -v test_multiprocessing_spawn

I appear to be somehow getting child processes where __main__.__file__ is set, 
but __main__.__spec__ is not.

--
nosy: +larry
priority: low -> release blocker
Added file: 
http://bugs.python.org/file33146/issue19946_pep_451_multiprocessing.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19946] Handle a non-importable __main__ in multiprocessing

[Richard Oudkerk]

Richard Oudkerk added the comment:

So there are really two situations:

1) The __main__ module *should not* be imported.  This is the case if you use 
__main__.py in a package or if you use nose to call test_main().

This should really be detected in get_preparation_data() in the parent process 
so that import_main_path() does not get called in the child process.

2) The __main__ module *should* be imported but it does not have a .py 
extension.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19946] Handle a non-importable __main__ in multiprocessing

[Nick Coghlan]

Nick Coghlan added the comment:

Issue 19700 isn't quite finished, but I believe it is finished enough to let me 
get this working properly.

--
dependencies:  -Update runpy for PEP 451
resolution: fixed -> 

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19946] Handle a non-importable __main__ in multiprocessing

[Nick Coghlan]

Changes by Nick Coghlan :


--
assignee:  -> ncoghlan

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19946] Handle a non-importable __main__ in multiprocessing

[Nick Coghlan]

Changes by Nick Coghlan :


--
title: Have multiprocessing raise ImportError when spawning a process that 
can't find the "main" module -> Handle a non-importable __main__ in 
multiprocessing

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com