This repository has been archived by the owner on Nov 10, 2023. It is now read-only.
make allocators and sanitizers work for processes created with multiprocessing's spawn method in dev mode #2660
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This pull request was exported from Phabricator. Differential Revision: D31106794 |
yifuwang
force-pushed
the
export-D31106794-to-dev
branch
from
f2b6ab2 to
5e4c739
Compare
|
This pull request was exported from Phabricator. Differential Revision: D31106794 |
yifuwang
added a commit
to yifuwang/buck
that referenced
this pull request
Sep 28, 2021
…rocessing's spawn method in dev mode (facebook#2660) Summary: Pull Request resolved: facebook#2660 #### Problem Currently, the entrypoint for in-place Python binaries (i.e. built with dev mode) executes the following steps to load system native dependencies (e.g. sanitizers and allocators): - Backup `LD_PRELOAD` set by the caller - Append system native dependencies to `LD_PRELOAD` - Inject a prologue in user code which restores `LD_PRELOAD` set by the caller - `execv` Python interpreter The steps work as intended for single process Python programs. However, when a Python program spawns child processes, the child processes will not load native dependencies, since they simply `execv`'s the vanilla Python interpreter. A few examples why this is problematic: - The ASAN runtime library is a system native dependency. Without loading it, a child process that loads user native dependencies compiled with ASAN will crash during static initialization because it can't find `_asan_init`. - `jemalloc` is also a system native dependency. Many if not most ML use cases "bans" dev mode because of these problems. It is very unfortunate considering the developer efficiency dev mode provides. In addition, a huge amount of unit tests have to run in a more expensive build mode because of these problems. #### Solution Move the system native dependencies loading logic out of the Python binary entrypoint into an interpreter wrapper, and set the interpreter as `sys.executable` in the injected prologue: - The Python binary entrypoint now uses the interpreter wrapper, which has the same command line interface as the Python interpreter, to run the main module. - `multiprocessing`'s `spawn` method now uses the interpreter wrapper to create child processes, ensuring system native dependencies get loaded correctly. #### Alternative Considered One alternative considered is to simply not removing system native dependencies from `LD_PRELOAD`, so they are present in the spawned processes. However, this can cause some linking issues, which were perhaps the reason `LD_PRELOAD` was restored in the first place. Reviewed By: fried fbshipit-source-id: aecfd839a77d1c395c3f7efbf1970698bf4b8016
|
This pull request was exported from Phabricator. Differential Revision: D31106794 |
yifuwang
force-pushed
the
export-D31106794-to-dev
branch
2 times, most recently
from
10d6863 to
f19bfdb
Compare
|
This pull request was exported from Phabricator. Differential Revision: D31106794 |
yifuwang
added a commit
to yifuwang/buck
that referenced
this pull request
Sep 30, 2021
…rocessing's spawn method in dev mode (facebook#2660) Summary: Pull Request resolved: facebook#2660 **The first attempt (D30802446) overlooked that fact that the interpreter wrapper is not an executable (for `execv`) on Mac, and introduced some bugs due to the refactoring. The attempt 2 addressed the issues, and isolated the effect of the change to only processes created by multiprocess's spawn method on Linux.** #### Problem Currently, the entrypoint for in-place Python binaries (i.e. built with dev mode) executes the following steps to load system native dependencies (e.g. sanitizers and allocators): - Backup `LD_PRELOAD` set by the caller - Append system native dependencies to `LD_PRELOAD` - Inject a prologue in user code which restores `LD_PRELOAD` set by the caller - `execv` Python interpreter The steps work as intended for single process Python programs. However, when a Python program spawns child processes, the child processes will not load native dependencies, since they simply `execv`'s the vanilla Python interpreter. A few examples why this is problematic: - The ASAN runtime library is a system native dependency. Without loading it, a child process that loads user native dependencies compiled with ASAN will crash during static initialization because it can't find `_asan_init`. - `jemalloc` is also a system native dependency. Many if not most ML use cases "bans" dev mode because of these problems. It is very unfortunate considering the developer efficiency dev mode provides. In addition, a huge amount of unit tests have to run in a more expensive build mode because of these problems. #### Solution Move the system native dependencies loading logic out of the Python binary entrypoint into an interpreter wrapper, and set the interpreter as `sys.executable` in the injected prologue: - The Python binary entrypoint now uses the interpreter wrapper, which has the same command line interface as the Python interpreter, to run the main module. - `multiprocessing`'s `spawn` method now uses the interpreter wrapper to create child processes, ensuring system native dependencies get loaded correctly. #### Alternative Considered One alternative considered is to simply not removing system native dependencies from `LD_PRELOAD`, so they are present in the spawned processes. However, this can cause some linking issues, which were perhaps the reason `LD_PRELOAD` was restored in the first place. Reviewed By: fried fbshipit-source-id: e3f27dc66894a4a3fa13f2d725a4eb43ced41451
|
This pull request was exported from Phabricator. Differential Revision: D31106794 |
yifuwang
force-pushed
the
export-D31106794-to-dev
branch
from
f19bfdb to
945fbd3
Compare
…rocessing's spawn method in dev mode (facebook#2660) Summary: Pull Request resolved: facebook#2660 **The first attempt (D30802446) overlooked that fact that the interpreter wrapper is not an executable (for `execv`) on Mac, and introduced some bugs due to the refactoring. The attempt 2 addressed the issues, and isolated the effect of the change to only processes created by multiprocess's spawn method on Linux.** #### Problem Currently, the entrypoint for in-place Python binaries (i.e. built with dev mode) executes the following steps to load system native dependencies (e.g. sanitizers and allocators): - Backup `LD_PRELOAD` set by the caller - Append system native dependencies to `LD_PRELOAD` - Inject a prologue in user code which restores `LD_PRELOAD` set by the caller - `execv` Python interpreter The steps work as intended for single process Python programs. However, when a Python program spawns child processes, the child processes will not load native dependencies, since they simply `execv`'s the vanilla Python interpreter. A few examples why this is problematic: - The ASAN runtime library is a system native dependency. Without loading it, a child process that loads user native dependencies compiled with ASAN will crash during static initialization because it can't find `_asan_init`. - `jemalloc` is also a system native dependency. Many if not most ML use cases "bans" dev mode because of these problems. It is very unfortunate considering the developer efficiency dev mode provides. In addition, a huge amount of unit tests have to run in a more expensive build mode because of these problems. #### Solution Move the system native dependencies loading logic out of the Python binary entrypoint into an interpreter wrapper, and set the interpreter as `sys.executable` in the injected prologue: - The Python binary entrypoint now uses the interpreter wrapper, which has the same command line interface as the Python interpreter, to run the main module. - `multiprocessing`'s `spawn` method now uses the interpreter wrapper to create child processes, ensuring system native dependencies get loaded correctly. #### Alternative Considered One alternative considered is to simply not removing system native dependencies from `LD_PRELOAD`, so they are present in the spawned processes. However, this can cause some linking issues, which were perhaps the reason `LD_PRELOAD` was restored in the first place. Reviewed By: fried, Reubend fbshipit-source-id: 9528c1856bf389ce033a8630cd718466754f3cef
yifuwang
force-pushed
the
export-D31106794-to-dev
branch
from
945fbd3 to
7faa8a5
Compare
|
This pull request was exported from Phabricator. Differential Revision: D31106794 |
facebook-github-bot
pushed a commit
that referenced
this pull request
Oct 13, 2021
…rocessing's spawn method in dev mode (#2660) Summary: Pull Request resolved: #2660 **The first attempt (D30802446) overlooked that fact that the interpreter wrapper is not an executable (for `execv`) on Mac, and introduced some bugs due to the refactoring. The attempt 2 addressed the issues, and isolated the effect of the change to only processes created by multiprocess's spawn method on Linux.** #### Problem Currently, the entrypoint for in-place Python binaries (i.e. built with dev mode) executes the following steps to load system native dependencies (e.g. sanitizers and allocators): - Backup `LD_PRELOAD` set by the caller - Append system native dependencies to `LD_PRELOAD` - Inject a prologue in user code which restores `LD_PRELOAD` set by the caller - `execv` Python interpreter The steps work as intended for single process Python programs. However, when a Python program spawns child processes, the child processes will not load native dependencies, since they simply `execv`'s the vanilla Python interpreter. A few examples why this is problematic: - The ASAN runtime library is a system native dependency. Without loading it, a child process that loads user native dependencies compiled with ASAN will crash during static initialization because it can't find `_asan_init`. - `jemalloc` is also a system native dependency. Many if not most ML use cases "bans" dev mode because of these problems. It is very unfortunate considering the developer efficiency dev mode provides. In addition, a huge amount of unit tests have to run in a more expensive build mode because of these problems. #### Solution Move the system native dependencies loading logic out of the Python binary entrypoint into an interpreter wrapper, and set the interpreter as `sys.executable` in the injected prologue: - The Python binary entrypoint now uses the interpreter wrapper, which has the same command line interface as the Python interpreter, to run the main module. - `multiprocessing`'s `spawn` method now uses the interpreter wrapper to create child processes, ensuring system native dependencies get loaded correctly. #### Alternative Considered One alternative considered is to simply not removing system native dependencies from `LD_PRELOAD`, so they are present in the spawned processes. However, this can cause some linking issues, which were perhaps the reason `LD_PRELOAD` was restored in the first place. Reviewed By: fried, Reubend fbshipit-source-id: fe75db57c3067eac35104c7a368f6bffde1ac50f
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
The first attempt (D30802446) overlooked that fact that the interpreter
wrapper is not an executable (for
execv) on Mac, and introduced some bugs dueto the refactoring. The attempt 2 addressed the issues, and isolated the effect
of the change to only processes created by multiprocess's spawn method on
Linux.
Problem
Currently, the entrypoint for in-place Python binaries (i.e. built with dev
mode) executes the following steps to load system native dependencies (e.g.
sanitizers and allocators):
LD_PRELOADset by the callerLD_PRELOADLD_PRELOADset by the callerexecvPython interpreterThe steps work as intended for single process Python programs. However, when a
Python program spawns child processes, the child processes will not load native
dependencies, since they simply
execv's the vanilla Python interpreter. A fewexamples why this is problematic:
child process that loads user native dependencies compiled with ASAN will
crash during static initialization because it can't find
_asan_init.jemallocis also a system native dependency.Many if not most ML use cases "bans" dev mode because of these problems. It is
very unfortunate considering the developer efficiency dev mode provides. In
addition, a huge amount of unit tests have to run in a more expensive build
mode because of these problems.
For an earlier discussion, see this post.
Solution
Move the system native dependencies loading logic out of the Python binary
entrypoint into an interpreter wrapper, and set the interpreter as
sys.executablein the injected prologue:same command line interface as the Python interpreter, to run the main
module.
multiprocessing'sspawnmethod now uses the interpreter wrapper to createchild processes, ensuring system native dependencies get loaded correctly.
Alternative Considered
One alternative considered is to simply not removing system native dependencies
from
LD_PRELOAD, so they are present in the spawned processes. However, thiscan cause some linking issues, which were perhaps the reason
LD_PRELOADwasrestored in the first place.
References
An old RFC for this change: D16210828
The counterpart for opt mode: D16350169
Reviewed By: fried