New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-32146: multiprocessing freeze_support needed outside win32 #5195

Open
wants to merge 15 commits into
base: master
from

Conversation

Projects
None yet
6 participants
@bbayles
Copy link
Contributor

bbayles commented Jan 15, 2018

This PR un-breaks "frozen" executables (e.g., those produced by PyInstaller or cx_Freeze that (a) run on Unix, and (b) use multiprocessing, and (c) use the spawn or forkserver start methods.

I linked to this dissection of the problem in the issue; it is a good overview of what goes wrong currently. In a nutshell, the forkserver and semaphore_tracker modules try to launch new processes with command line arguments that the "frozen" executable doesn't support:

When using argparse, the error is clear:

error: unrecognized arguments: -S -E -c from multiprocessing.semaphore_tracker import main;main(5)

error: unrecognized arguments: -S -E -c from multiprocessing.forkserver import main; main(8, 9, ['__main__'], **{'sys_path': [''/opt/freeze_support_test/lib/library.zip', '/opt/freeze_support_test/lib']})

On Windows this problem is avoided because freeze_support captures the arguments, parses them, executes the appropriate code, and then exits here. It recognizes this by way of a dummy command line argument, --multiprocessing-fork.

This PR:

  • Removes the win32 check that prevents freeze_support from having an effect on Unix
  • Adds dummy --multiprocessing-forkserver and --multiprocessing-semaphore-tracker command line arguments, which are only used with "frozen" executables
  • Extends freeze_support such that it captures and parses the command line arguments for the fork server and semaphore tracker, and launches them.

This fixes the problem for me on both PyInstaller and cx_Freeze on Unix.

I couldn't find that there were any tests for the existing freeze_support code. I added some basic ones for the command line manipulation that I've added here, but it may be useful to file an issue to add others.

I will add a few more comments in-line. I'm a new-ish contributor, so if discussion of the approach to fixing this issue should take place elsewhere (in the bug tracker or on a mailing list), please let me know (gently). Many thanks!

https://bugs.python.org/issue32146

return None

# command ends with main(fd) - extract fd
r = int(argv[-1].rsplit('(')[1].split(')')[0])

This comment has been minimized.

@bbayles

bbayles Jan 15, 2018

Contributor

Here we are parsing what gets produced in semaphore_tracker.SemaphoreTracker.ensure_running. It goes to semaphore_tracker.main - a single integer.

# listener_fd and alive_r are integers
# preload is a list
# kwds map strings to lists
main_args = argv[-1].split('main(')[1].rsplit(')', 1)[0].split(', ', 3)

This comment has been minimized.

@bbayles

bbayles Jan 15, 2018

Contributor

Here we are parsing a more difficult expression, which is produced by forkserver.ForkServer.ensure_running. It goes to forkserver.main.

The listener_fd and alive_r arguments are integers. The preload argument is a list, hence I have introduced ast.literal_eval.

The keyword arguments come to us as **{'sys_path': ['list_of_paths'], 'main_path': ['list_of_paths']}. I used literal_eval here as the least awkward option.

This comment has been minimized.

@bbayles

bbayles Jan 16, 2018

Contributor

An alternative to the splitting stuff, since ast is already in the mix:

parsed_cmd = ast.parse(argv[-1])
args = [ast.literal_eval(parsed_cmd.body[1].value.args[i]) for i in range(3)]
kwds = ast.literal_eval(parsed_cmd.body[1].value.keywords[0].value)

Needed in the "multiple preload modules" case.

@bbayles bbayles requested a review from 1st1 as a code owner Jan 29, 2018

@bbayles

This comment has been minimized.

Copy link
Contributor

bbayles commented Jan 29, 2018

Oops, I didn't mean to request that review. Apologies!

@1st1 1st1 removed their request for review Jan 29, 2018

@1st1

This comment has been minimized.

Copy link
Member

1st1 commented Jan 29, 2018

Oops, I didn't mean to request that review. Apologies!

NP, it happened automatically and you couldn't do anything about that even if you wanted to :-)

@ned-deily

This comment has been minimized.

Copy link
Member

ned-deily commented Jun 8, 2018

@applio Would you be willing to review this?

@bbayles

This comment has been minimized.

Copy link
Contributor

bbayles commented Jun 8, 2018

FYI, PR #5850 adds a note about this to the docs.

It would probably good to that one in for 3.7. I expect this PR will go through some revisions, but the docs one should be easy.

bbayles added some commits Jul 13, 2018

@vstinner

This comment has been minimized.

Copy link
Member

vstinner commented Jan 10, 2019

@pitrou yet another multiprocessing change ;-) It's also related to https://bugs.python.org/issue32146

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment