Skip to content
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

Use of concurrent.futures.process results in BrokenProcessPool exception #111

Closed
yulqen opened this issue Aug 5, 2019 · 5 comments
Closed
Labels
bug Something isn't working

Comments

@yulqen
Copy link

yulqen commented Aug 5, 2019

Successfully built my project in Windows 10 (woo!) with very little ceremony compared to other tools. Great work!

Unfortunately, I am getting the following exception when running code which makes use of futures.ProcessPoolExecutor:

My function is:

def extract_from_multiple_xlsx_files(xlsx_files):
    "Extract raw data from list of paths to excel files. Return as complex dictionary."
    data = {}
    with futures.ProcessPoolExecutor() as pool:
        for file in pool.map(template_reader, xlsx_files):
            data.update(file)
    return data

Exception raised when called:

Usage: bcompiler.exe [OPTIONS] COMMAND [ARGS]...
Try "bcompiler.exe --help" for help.

Error: no such option: -B
Usage: bcompiler.exe [OPTIONS] COMMAND [ARGS]...
Try "bcompiler.exe --help" for help.

Usage: bcompiler.exe [OPTIONS] COMMAND [ARGS]...
Try "bcompiler.exe --help" for help.

Error: no such option: -B
Error: no such option: -B
Usage: bcompiler.exe [OPTIONS] COMMAND [ARGS]...
Try "bcompiler.exe --help" for help.

Error: no such option: -B
Usage: bcompiler.exe [OPTIONS] COMMAND [ARGS]...
Try "bcompiler.exe --help" for help.

Error: no such option: -B
Usage: bcompiler.exe [OPTIONS] COMMAND [ARGS]...
Try "bcompiler.exe --help" for help.

Usage: bcompiler.exe [OPTIONS] COMMAND [ARGS]...
Try "bcompiler.exe --help" for help.

Error: no such option: -B
Error: no such option: -B
Usage: bcompiler.exe [OPTIONS] COMMAND [ARGS]...
Try "bcompiler.exe --help" for help.

Error: no such option: -B
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "click.core", line 764, in __call__
  File "click.core", line 717, in main
  File "click.core", line 1137, in invoke
  File "click.core", line 1137, in invoke
  File "click.core", line 956, in invoke
  File "click.core", line 555, in invoke
  File "bcompiler.main", line 79, in templates
  File "engine.adapters.cli", line 37, in import_and_create_master
  File "engine.use_cases.parsing", line 181, in execute
  File "engine.use_cases.parsing", line 138, in execute
  File "engine.use_cases.parsing", line 128, in _get_datamap_and_template_data
  File "engine.use_cases.parsing", line 77, in execute
  File "engine.repository.templates", line 40, in list_as_json
  File "engine.use_cases.parsing", line 368, in extract_from_multiple_xlsx_files
  File "concurrent.futures.process", line 483, in _chain_from_iterable_of_lists
  File "concurrent.futures._base", line 598, in result_iterator
  File "concurrent.futures._base", line 435, in result
  File "concurrent.futures._base", line 384, in __get_result
concurrent.futures.process.BrokenProcessPool: A process in the process pool was terminated abruptly while the future was running or pending.

There is no -B option in my application, so what is going on there!? I suspect there is something going on with the command line options parsing here, and perhaps this has nothing to do with multiprocessing, but I don't know how to debug.

The same codes runs fine on the same same system when installed via pip, etc. Posts elsewhere (such as here suggest that multiprocess code such as this needs to be protected with the if __name__ == '__main__' construction, but I don't see how this could apply in this case as the code is proven to work when installed normally.

Thanks for looking.

@indygreg
Copy link
Owner

indygreg commented Aug 6, 2019

I seem to recall multiprocessing doing weird things with regards to looking at the name/arguments of the current executable. It is quite possible PyOxidizer is violating some assumption made in the multiprocessing code. Adding this to the backlog of compatibility issues to look into.

@orn688
Copy link

orn688 commented Feb 3, 2021

FWIW I believe this issue breaks the documentation example of using Pyoxidizer to build Black (specifically with the latest version of black, not with version 19 as used in the example) because the latest version of black uses multiprocessing when run against a directory (e.g. black . rather than black foo.py).

@orn688
Copy link

orn688 commented Mar 28, 2021

I spent some time digging into this, and the problem is indeed the assumptions made by multiprocessing.

multiprocessing supports multiple methods of launching the processes that make up the process pool. "spawn" is the only supported option on Windows, and it involves launching a new Python interpeter as a subprocess to run the multiprocessing worker code (code reference). In order to launch the Python interpreter, multiprocessing looks up the path to the Python interpreter that's running the code, via sys.executable. However, with code running under PyOxidizer and similar tools, sys.executable is actually the path to the PyOxidizer-built binary, not to a Python interpreter.

That's why you're getting the Error: no such option: -B warning: you're running your code with .pyc file generation disabled (maybe with PYTHONDONTWRITEBYTECODE set), so multiprocessing runs the subprocess with the same configuration; -B is the command-line flag for disabling .pyc file generation. multiprocessing expects that sys.executable will be a path to a Python interpeter that supports the -B flag, but instead it's a path to your PyOxidizer-built executable, which doesn't support a -B flag.

I think the solution for Windows is to call multiprocessing.freeze_support() immediately when your code starts (e.g. at the beginning of if __name__ == "__main__"). freeze_support is a hack that will make the multiprocessing subprocesses do the correct thing and run the worker code, rather than running your application's code.

The misbehavior of "spawn" is also a problem on Linux and Mac, tracked by this Python bug. You can work around it on Linux and Mac by calling multiprocessing.set_start_method("fork"). "fork" will cause multiprocessing to start workers by forking the main process, so there's no need to know the path to a Python executable. "fork" is already the default on Linux, but "spawn" is still the default on Mac because forks can fail unexpectedly on Mac – see https://bugs.python.org/issue33725.

So to conclude, I actually don't think this is a bug in PyOxidizer, at least not one that's feasible to fix. Instead, it's a known limitation of PyOxidizer-like tools that embed a Python interpreter in an executable (otherwise known as "freezing"), and it seems like Python intends to maintain support for this use case. So I think this is just a matter of waiting for the aforementioned bug to be fixed so freeze_support() works on Unix, which will make the "spawn" subprocess start method work even under PyOxidizer.

I guess the one potential PyOxidizer improvement would be to document this limitation and the workaround that I mentioned, i.e. adding this to your application code:

import multiprocessing

...

if __name__ == "__main__":
    if "fork" in multiprocessing.get_all_start_methods():
        multiprocessing.set_start_method("fork")  # Handles Mac/Linux
    multiprocessing.freeze_support()  # Handles Windows
    rest_of_your_code_here()

Although that might not be safe on Mac due to the fork issue – I'm still not entirely clear on what kinds of applications it affects.

@indygreg
Copy link
Owner

Thank you so much for the investigation, @orn688! I definitely learned a lot about some multiprocessing gotchas.

My read of https://bugs.python.org/issue33725 indicates the macOS multiprocessing.set_start_method("fork") bug is only present when Python is compiled as a framework. Fortunately, our Python distributions are not framework builds, so we're not subject to this bug! That means we should be able to safely call multiprocessing.set_start_method("fork") on non-Windows environments.

The multiprocessing.freeze_support() is a bit more complicated. It looks like the Python standard library checks for sys.frozen = True, so we need to ensure that is set.

It looks like all that multiprocessing.freeze_support() does is look for sys.argv[1] == "--multiprocessing-fork", which appears to be a sentinel value, indicating we are in "multiprocessing mode." If we are, it takes over the process and runs the multiprocessing worker code.

What appears to happen on Windows is that a new instance of the current process is started with the aforementioned --multiprocessing-fork sentinel argument, plus details of what work to perform.

Theoretically, we should be able to look for --multiprocessing-fork immediately after Python interpreter startup and call into multiprocessing.freeze_support() if seen.

I think we can do all of this automatically. We may want some config knobs to control behavior. But I'm optimistic that we can get multiprocessing to just work by doing the following:

  1. Automatically calling multiprocessing.set_start_method("fork") on import of multiprocessing on platforms that support forking.
  2. Checking for sys.argv[1] == "--multiprocessing-fork" immediately after interpreter startup when sys.frozen == True and calling multiprocessing.freeze_support() automatically.

Applications would still need to set sys.frozen = True via a pyoxidizer.bzl config knob for Windows support. But I've been thinking about making this the default. Having applications that just work on Windows seems like a compelling reason to do that.

indygreg added a commit that referenced this issue May 16, 2021
…ally

pyembed is taught to call multiprocessing.set_start_method()
automatically when servicing the import of the multiprocessing module.

Support for this configurable behavior is wired through to Starlark.

On macOS, we use the "fork" method (unlike Python itself) because
we don't use framework Python builds and aren't subject to the
limitation of fork() not working in framework code.

multiprocessing still doesn't "just work" on Windows. That will
require additional feature work.

Part of #111.
@indygreg
Copy link
Owner

I believe the main branch now has multiprocessing support that just works, assuming the default interpreter configuration is used.

I've tested this on Linux, macOS, and Windows and can confirm that processes launched with multiprocessing appear to just work, with application not needing to do anything special to enable this.

This is documented at https://pyoxidizer.readthedocs.io/en/latest/pyoxidizer_packaging_multiprocessing.html and will be released in 0.17.

I would appreciate if others could test the new behavior and report back experiences.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants