Fix issues with forking on Cygwin #1450
Merged
+41
−16
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
…ate platforms (including Cygwin, which has fork())
Even if we're directly using the win32 threading driver and not pthreads, pthread_atfork still works fine to register a pre-fork handler, and is necessary to restore the threading server to a pre-initialized state.
…ich are not fork-safe.
|
Thanks! |
|
Fork test seems to hang on ARMv8 as well, probably better to enable it only on systems where we know it should pass... |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
In most places OpenBLAS treats Cygwin as equivalent to Windows, and in most cases this is fine except when it comes to forking a process that's using OpenBLAS (since native Windows does not know about Cygwin's fork()).
In particular, the win32 threading module is broken after fork, and should be shut down before fork, so we must support adding a
pthread_atforkhook for this on Cygwin. Note, this is still using the native win32 threading module even on Cygwin--in principle Cygwin could use pthreads instead, but the win32 threading actually works just fine and probably involves less overhead. It just needs the additional care around fork.What does not work around fork, is using the native Windows memory allocation functions on Cygwin, so this patch also changes memory allocation to use mmap on Cygwin instead.