From 6ba7852a62e875b2debb07f8deff19c59ddd3e02 Mon Sep 17 00:00:00 2001 From: Joel Dice Date: Wed, 16 Jan 2008 13:46:39 -0700 Subject: [PATCH] tweak System::Monitor::wait to avoid notify deadlock --- src/posix.cpp | 72 +++++++++++++++++++++------------------- src/windows.cpp | 87 ++++++++++++++++++++++++++----------------------- 2 files changed, 86 insertions(+), 73 deletions(-) diff --git a/src/posix.cpp b/src/posix.cpp index b0a5c3c404..44a5080721 100644 --- a/src/posix.cpp +++ b/src/posix.cpp @@ -274,50 +274,56 @@ class MySystem: public System { Thread* t = static_cast(context); if (owner_ == t) { - ACQUIRE(t->mutex); + bool interrupted; + unsigned depth; + + { ACQUIRE(t->mutex); - if (t->r->interrupted()) { - t->r->setInterrupted(false); - return true; - } + if (t->r->interrupted()) { + t->r->setInterrupted(false); + return true; + } - t->flags |= Waiting; + t->flags |= Waiting; - append(t); + append(t); - unsigned depth = this->depth; - this->depth = 0; - owner_ = 0; - pthread_mutex_unlock(&mutex); + depth = this->depth; + this->depth = 0; + owner_ = 0; + pthread_mutex_unlock(&mutex); - if (time) { - int64_t then = s->now() + time; - timespec ts = { then / 1000, (then % 1000) * 1000 * 1000 }; - int rv UNUSED = pthread_cond_timedwait - (&(t->condition), &(t->mutex), &ts); - expect(s, rv == 0 or rv == ETIMEDOUT or rv == EINTR); - } else { - int rv UNUSED = pthread_cond_wait(&(t->condition), &(t->mutex)); - expect(s, rv == 0 or rv == EINTR); + if (time) { + int64_t then = s->now() + time; + timespec ts = { then / 1000, (then % 1000) * 1000 * 1000 }; + int rv UNUSED = pthread_cond_timedwait + (&(t->condition), &(t->mutex), &ts); + expect(s, rv == 0 or rv == ETIMEDOUT or rv == EINTR); + } else { + int rv UNUSED = pthread_cond_wait(&(t->condition), &(t->mutex)); + expect(s, rv == 0 or rv == EINTR); + } + + if ((t->flags & Notified) == 0) { + remove(t); + } + + t->flags = 0; + t->next = 0; + + if (t->r->interrupted()) { + t->r->setInterrupted(false); + interrupted = true; + } else { + interrupted = false; + } } pthread_mutex_lock(&mutex); owner_ = t; this->depth = depth; - - if ((t->flags & Notified) == 0) { - remove(t); - } - t->flags = 0; - t->next = 0; - - if (t->r->interrupted()) { - t->r->setInterrupted(false); - return true; - } else { - return false; - } + return interrupted; } else { sysAbort(s); } diff --git a/src/windows.cpp b/src/windows.cpp index 0b27fb6083..2eb1a70c68 100644 --- a/src/windows.cpp +++ b/src/windows.cpp @@ -240,55 +240,62 @@ class MySystem: public System { assert(s, t); if (owner_ == t) { - ACQUIRE(s, t->mutex); + bool interrupted; + unsigned depth; + int r UNUSED; + + { ACQUIRE(s, t->mutex); - if (t->r->interrupted()) { - t->r->setInterrupted(false); - return true; + if (t->r->interrupted()) { + t->r->setInterrupted(false); + return true; + } + + t->flags |= Waiting; + + append(t); + + depth = this->depth; + this->depth = 0; + owner_ = 0; + + bool success UNUSED = ReleaseMutex(mutex); + assert(s, success); + + success = ResetEvent(t->event); + assert(s, success); + + success = ReleaseMutex(t->mutex); + assert(s, success); + + r = WaitForSingleObject(t->event, (time ? time : INFINITE)); + assert(s, r == WAIT_OBJECT_0 or r == WAIT_TIMEOUT); + + r = WaitForSingleObject(t->mutex, INFINITE); + assert(s, r == WAIT_OBJECT_0); + + if ((t->flags & Notified) == 0) { + remove(t); + } + + t->flags = 0; + t->next = 0; + + if (t->r->interrupted()) { + t->r->setInterrupted(false); + interrupted = true; + } else { + interrupted = false; + } } - t->flags |= Waiting; - - append(t); - - unsigned depth = this->depth; - this->depth = 0; - owner_ = 0; - - bool success UNUSED = ReleaseMutex(mutex); - assert(s, success); - - success = ResetEvent(t->event); - assert(s, success); - - success = ReleaseMutex(t->mutex); - assert(s, success); - - int r UNUSED = WaitForSingleObject(t->event, (time ? time : INFINITE)); - assert(s, r == WAIT_OBJECT_0 or r == WAIT_TIMEOUT); - - r = WaitForSingleObject(t->mutex, INFINITE); - assert(s, r == WAIT_OBJECT_0); - r = WaitForSingleObject(mutex, INFINITE); assert(s, r == WAIT_OBJECT_0); owner_ = t; this->depth = depth; - - if ((t->flags & Notified) == 0) { - remove(t); - } - t->flags = 0; - t->next = 0; - - if (t->r->interrupted()) { - t->r->setInterrupted(false); - return true; - } else { - return false; - } + return interrupted; } else { sysAbort(s); }