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

rhtooks: multiprocessing: use class-wide lock to prevent race conditions #7411

Merged
merged 1 commit into from Feb 7, 2023

Conversation

rokm
Copy link
Member

@rokm rokm commented Feb 1, 2023

Use class-wide lock in the FrozenSupportMixIn constructor in multiprocessing run-time hook to prevent race conditions between os.putenv and os.unsetenv calls when processes are spawned concurrently from multiple threads. This essentially serializes process spawning and its associated environment modifications (which are done in main process and inherited by child ones).

Fixes #7410.

Also get rid of redunant check for sys.frozen.

Use class-wide lock in the `FrozenSupportMixIn` constructor in
`multiprocessing` run-time hook to  prevent race conditions between
`os.putenv` and `os.unsetenv` calls when processes are spawned
concurrently from multiple threads. This essentially serializes
process spawning and its associated environment modifications
(which are done in main process and inherited by child ones).

Also get rid of redunant check for `sys.frozen`.
@rokm rokm marked this pull request as ready for review February 5, 2023 15:48
@rokm rokm requested a review from bwoodsend February 5, 2023 15:49
@rokm
Copy link
Member Author

rokm commented Feb 5, 2023

I think a better, long-term solution for this would be to stop clearing the _MEIPASS2 environment variable.

To support spawning other PyInstaller-frozen applications from a frozen application, we should instead store path to PKG archive to another environment variable, and then use that in comparison to determine whether the _MEIPASS2 should be inherited (i.e., re-use the existing temporary directory and files that are extracted there) or re-set (extract files to new temporary directory).

But to avoid breaking the (unlikely?) scenario of having PyInstaller-frozen application with new mechanism run a PyInstaller-frozen application with old mechanism, we would probably have to rename the _MEIPASS2 variable to something else (maybe also something more descriptive, like PYI_TEMPORARY_DIR). So that the application with old mechanism does not end up with _MEIPASS2 set..

@rokm rokm merged commit 94475c3 into pyinstaller:develop Feb 7, 2023
@rokm rokm deleted the fix-multiprocessing-race-condition branch February 7, 2023 19:42
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multiprocessing "spawn" not thread-safe on Linux.
2 participants