[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date: Fri, 11 Mar 2022 12:41:05 -0700
From: Gavin Howard <gavin.d.howard@...il.com>
To: musl@...ts.openwall.com
Subject: Possible PEBKAC or Bug in musl Semaphores and/or Mutexes
Hello,
If this email seems rushed, it's because I was told musl is close to
being released and that you all would want to know about this bug
quickly.
All of this was tested on musl 1.2.2.
I'm writing a multi-threaded build system, and I've run into an
interesting error when I compile with musl (the musl-gcc generated by
building musl from source).
Background:
I have two main threads and as many worker threads as desired.
Worker Threads: Responsible for actually building targets. Once they are
spawned, they wait on a semaphore for targets and grab them (with a
mutex) off of a queue. When worker threads run a child process, they do
the following:
* Run the process (whether through posix_spawn or fork()+exec()).
* Check the return value to make sure the child process was created.
* Take a mutex.
* Add the child pid and an fd to communicate with the thread that is the
parent of the child to a map.
* Post a semaphore.
* Unlock the mutex.
* Continue executing the target and other targets.
Main Thread #1: Spawns all of the other threads, waits on the worker
threads to join, and once they do, does the following:
* Locks the mutex mentioned above.
* Sets a flag.
* Posts the semaphore mentioned above.
* Unlocks the mutex.
* Calls pthread_join() on Main Thread #2.
Main Thread #2: Responsible for sending info about all reaped children
to the threads that spawned them. Once it is started, it does the
following:
* Waits on the semaphore mentioned above.
* Locks the mutex mentioned above.
* Checks the flag mentioned above. If it is set, it unlocks the mutex
and does a pthread_exit().
* If the flag is not set, it continues and:
* Unlocks the mutex.
* Does a waitpid().
* Checks the return value of waitpid(). If it's ECHILD, it causes an
abort() (for the purposes of debugging this issue).
* Locks the mutex again.
* Sends the child status info to the right thread using the map
mentioned above.
* Unlocks the mutex.
* Repeat the process.
When running my code under musl and glibc, the abort() mentioned above
is triggered every so often at the end. This was because I was using an
atomic variable with the semaphore. I could trigger the problem every
10-20 minutes by continually bootstrapping the build system, and it
always happened at the end of the build, when the semaphore was posted
to have Main Thread #2 exit.
The reason the atomic variable did not work is (I suspect) because there
is no guarantee of compiler or processor ordering with pure,
memory_order_relaxed atomic variables. I suspect that the processor was
reordering the instructions such that the semaphore was signalled by
Main Thread #1 before it set the atomic variable, even though the atomic
store to the variable was before the semaphore post call in the source.
The setting of the flag could then come *after* Main Thread #2 did an
atomic load on it.
So I replaced it with the scheme I laid out above, and on glibc, the
issue still triggers; it just takes about 4 hours to do it.
However, on musl, I can still consistently trigger the issue in less
than 10 minutes.
Nevertheless, I still think it is more correct, and here's why: the
mutex should enforce ordering in the processor. By holding the mutex
when the semaphore is posted, the post cannot be reordered to before the
taking of the mutex lock, or after releasing it. Then, because the flag
is also set while holding the mutex lock, this means that setting the
flag cannot be reordered to before the lock. This means that the setting
of the flag always comes before the checking of the flag, even if Main
Thread #2's wait on the semaphore ends *before* the lock is released.
This, in turn, means that when Main Thread #2 takes the lock after
waiting on the semaphore, the flag *will* be set if it has been set at
all.
Likewise, since a thread adding a process to the map holds the lock
while adding the process to the map, and posts the semaphore *while
holding the lock*, then if Main Thread #2's wait on the semaphore ends
before the lock is released, it is still forced to wait to take the
lock. This taking of the lock also forces the waitpid() call to happen
*after* the releasing of the lock by the thread that is adding a process
to the map.
In like manner, the taking and releasing of the lock by the worker
thread means that the creation of the child process must happen before
the release of the lock, so I believe that there should *never* be a
situation where waitpid() is called before a child process is properly
created.
I believe all of that together means that there should *never* be a
situation where waitpid() returns ECHILD. Yet musl does, consistently.
I've checked the musl source for waitpid(), and it's just a syscall, as
it should be, so I don't think the bug is in waitpid().
So after eliminating other possibilities, it seems to me there may be a
bug in either musl's semaphores, its mutexes, or both.
Now, I fully admit that I could have done my analysis wrong and still
have a bug in my code. I admit that this could still be PEBKAC. But even
in that case, I need the help of the musl community to figure out where
I did go wrong.
Thank you for your time.
Gavin Howard
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
