Fix race in blas_thread_shutdown. #3111
Conversation
blas_server_avail was read without holding server_lock. If multiple threads call blas_thread_shutdown simultaneously, for example, by calling fork(), then they can attempt to shut down multiple times. This can lead to a segmentation fault.
…/execvp() on non-Android POSIX platforms. The goal of this change is to avoid calling pthread_atfork() handlers. Some libraries, in particular the version of OpenBLAS included in NumPy, have buggy pthread_atfork() handlers. See OpenMathLib/OpenBLAS#3111 and jax-ml/jax#5713 for details. Now, while we can and have fixed the buggy atfork handlers, it will take some time for the fix to be deployed in a NumPy release and for users to update to a new NumPy release. So we also take an additional step: avoid running atfork handlers in Subprocess. My copy of the glibc documentation says: " According to POSIX, it unspecified whether fork handlers established with pthread_atfork(3) are called when posix_spawn() is invoked. On glibc, fork handlers are called only if the child is created using fork(2). " It appears glibc 2.24 and newer do not call pthread_atfork() handlers from posix_spawn(). Using posix_spawn() should be at least no worse than an explicit fork()/execvp() pair, and on glibc it should do the right thing. PiperOrigin-RevId: 358317859 Change-Id: Ic1d95446706efa7c0db4e79bf8281f14b2bd99df
|
Ouch, thanks. Don't know how often I (and certainly many others) have looked at that part of the code without realizing. |
|
Thanks! I should add: I'm pretty sure some of the other code patterns in that file related to For example, suppose another thread calls In general it looks fairly suspicious to me to read More broadly, I think it might be a mistake to shut down the parent's thread pool on fork; you most likely only really want to ensure that the child starts without a threadpool configured, and I think that could be done with a slightly more sophisticated |
blas_server_availwas read without holdingserver_lock. If multiple threads callblas_thread_shutdownsimultaneously, for example, by callingfork(), then they can attempt to shut down multiple times. This can lead to a segmentation fault.This may be the same issue as #2270 although in my case it exhibited as a segmentation fault, not a deadlock.