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

Fix issues with forking on Cygwin #1450

Merged
merged 3 commits into from Feb 8, 2018
Merged

Conversation

@embray
Copy link
Contributor

embray commented Feb 6, 2018

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_atfork hook 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.

embray added 3 commits Feb 6, 2018
…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.
@martin-frbg martin-frbg merged commit d31f62c into xianyi:develop Feb 8, 2018
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@embray embray deleted the embray:cygwin/forking branch Feb 12, 2018
@embray

This comment has been minimized.

Copy link
Contributor Author

embray commented Feb 12, 2018

Thanks!

@martin-frbg

This comment has been minimized.

Copy link
Collaborator

martin-frbg commented Feb 16, 2018

Fork test seems to hang on ARMv8 as well, probably better to enable it only on systems where we know it should pass...

martin-frbg added a commit that referenced this pull request Feb 7, 2019
* Restore dropped patches in the non-TLS branch of memory.c

As discovered in #2002, the reintroduction of the "original" non-TLS version of memory.c as an alternate branch had inadvertently used ba1f91f rather than a8002e2 , thereby dropping the commits for #1450, #1468, #1501, #1504 and #1520.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.