Skip to content

Commit

Permalink
Fix deadlock in stopAllThreads()
Browse files Browse the repository at this point in the history
stopAllThreads() and createThread() can deadlock (and sometimes did)
because the first function takes out the threadMapMux lock and then
tries to pthread_join() other threads.

createThread() also tries to take out the lock but if stopAllThreads()
owns it and another thread calls createThread(), then boom, the threads
would deadlock each other.  Drop the lock before calling pthread_join()
and reacquire it afterwards.

This problem was noticed on an AIX machine with Node.js Application
Metrics but since the Linux port uses identical logic, I've updated
that as well.  The MacOS port has that code commented out but that
didn't stop me from updating it anyway (but leaving it commented out.)
  • Loading branch information
bnoordhuis committed Oct 27, 2017
1 parent 919d63a commit a743bf0
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 6 deletions.
7 changes: 5 additions & 2 deletions src/ibmras/common/port/aix/Thread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,12 @@ void stopAllThreads() {
// wake currently sleeping threads
condBroadcast();
while (!threadMap.empty()) {
pthread_cancel(threadMap.top());
pthread_t top = threadMap.top();
pthread_cancel(top);
//wait for the thread to stop
pthread_join(threadMap.top(), NULL);
pthread_mutex_unlock(&threadMapMux);
pthread_join(top, NULL);
pthread_mutex_lock(&threadMapMux);
threadMap.pop();
}
pthread_mutex_unlock(&threadMapMux);
Expand Down
7 changes: 5 additions & 2 deletions src/ibmras/common/port/linux/Thread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,12 @@ void stopAllThreads() {
// wake currently sleeping threads
condBroadcast();
while (!threadMap.empty()) {
pthread_cancel(threadMap.top());
pthread_t top = threadMap.top();
pthread_cancel(top);
//wait for the thread to stop
pthread_join(threadMap.top(), NULL);
pthread_mutex_unlock(&threadMapMux);
pthread_join(top, NULL);
pthread_mutex_lock(&threadMapMux);
threadMap.pop();
}
pthread_mutex_unlock(&threadMapMux);
Expand Down
7 changes: 5 additions & 2 deletions src/ibmras/common/port/osx/Thread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,12 @@ void stopAllThreads() {
condBroadcast();
/*
while (!threadMap.empty()) {
pthread_cancel(threadMap.top());
pthread_t top = threadMap.top();
pthread_cancel(top);
//wait for the thread to stop
pthread_join(threadMap.top(), NULL);
pthread_mutex_unlock(&threadMapMux);
pthread_join(top, NULL);
pthread_mutex_lock(&threadMapMux);
threadMap.pop();
}
*/
Expand Down

0 comments on commit a743bf0

Please sign in to comment.