interrupt() / notify() fix.
authorStefan Ring <stefan@complang.tuwien.ac.at>
Mon, 3 Mar 2008 21:22:02 +0000 (22:22 +0100)
committerStefan Ring <stefan@complang.tuwien.ac.at>
Mon, 3 Mar 2008 21:22:02 +0000 (22:22 +0100)
* src/threads/native/lock.c (lock_record_wait, lock_record_notify):
Correctly handling the signaled and interrupted flags.

* src/threads/native/threads.c (threads_wait_with_timeout)
(threads_wait_with_timeout_relative): No return value, not touching flags.
(threads_check_if_interrupted_and_reset): Now locking the waitmutex.
(threads_sleep): Adapted to change.

* src/threads/native/threads.h (threads_wait_with_timeout_relative): No
return value.

src/threads/native/lock.c
src/threads/native/threads.c
src/threads/native/threads.h

index b7cf82117e5fa26bb414a90e2212447241bc3623..6bcc20294e84070f1cf89bdbe2216ab28c22088e 100644 (file)
@@ -1268,7 +1268,7 @@ static void lock_record_remove_waiter(lock_record_t *lr, threadobject *thread)
 static bool lock_record_wait(threadobject *thread, lock_record_t *lr, s8 millis, s4 nanos)
 {
        s4   lockcount;
-       bool wasinterrupted;
+       bool wasinterrupted = false;
 
        DEBUGLOCKS(("[lock_record_wait  : lr=%p, t=%p, millis=%lld, nanos=%d]",
                                lr, thread, millis, nanos));
@@ -1290,7 +1290,7 @@ static bool lock_record_wait(threadobject *thread, lock_record_t *lr, s8 millis,
 
        /* wait until notified/interrupted/timed out */
 
-       wasinterrupted = threads_wait_with_timeout_relative(thread, millis, nanos);
+       threads_wait_with_timeout_relative(thread, millis, nanos);
 
        /* re-enter the monitor */
 
@@ -1304,6 +1304,17 @@ static bool lock_record_wait(threadobject *thread, lock_record_t *lr, s8 millis,
 
        lr->count = lockcount;
 
+       /* We can only be signaled OR interrupted, not both. If both flags
+          are set, reset only signaled and leave the thread in
+          interrupted state. Otherwise, clear both. */
+
+       if (!thread->signaled) {
+               wasinterrupted = thread->interrupted;
+               thread->interrupted = false;
+       }
+
+       thread->signaled = false;
+
        /* return if we have been interrupted */
 
        return wasinterrupted;
@@ -1401,10 +1412,10 @@ static void lock_record_notify(threadobject *t, lock_record_t *lr, bool one)
 
                waitingthread = w->thread;
 
-               /* If the thread was already signaled but hasn't removed
-                  itself from the list yet, just ignore it. */
+               /* We must skip threads which have already been notified or
+                  interrupted. They will remove themselves from the list. */
 
-               if (waitingthread->signaled == true)
+               if (waitingthread->signaled || waitingthread->interrupted)
                        continue;
 
                /* Enter the wait-mutex. */
@@ -1414,7 +1425,11 @@ static void lock_record_notify(threadobject *t, lock_record_t *lr, bool one)
                DEBUGLOCKS(("[lock_record_notify: lr=%p, t=%p, waitingthread=%p, sleeping=%d, one=%d]",
                                        lr, t, waitingthread, waitingthread->sleeping, one));
 
-               /* Signal the thread if it's sleeping. */
+               /* Signal the thread if it's sleeping. sleeping can be false
+                  when the waiting thread is blocked between giving up the
+                  monitor and entering the waitmutex. It will eventually
+                  observe that it's signaled and refrain from going to
+                  sleep. */
 
                if (waitingthread->sleeping)
                        pthread_cond_signal(&(waitingthread->waitcond));
index ce18dfecd3b753477bbe1b0edf5bf708f9a0a098..f63abbfd638435170b7fa4d294255dca04ac9a03 100644 (file)
@@ -2052,16 +2052,10 @@ static bool threads_current_time_is_earlier_than(const struct timespec *tv)
                           If both tv_sec and tv_nsec are zero, this function
                                           waits for an unlimited amount of time.
 
-   RETURN VALUE:
-      true.........if the wait has been interrupted,
-         false........if the wait was ended by notification or timeout
-
 *******************************************************************************/
 
-static bool threads_wait_with_timeout(threadobject *t, struct timespec *wakeupTime)
+static void threads_wait_with_timeout(threadobject *t, struct timespec *wakeupTime)
 {
-       bool wasinterrupted;
-
        /* acquire the waitmutex */
 
        pthread_mutex_lock(&t->waitmutex);
@@ -2096,21 +2090,11 @@ static bool threads_wait_with_timeout(threadobject *t, struct timespec *wakeupTi
                }
        }
 
-       /* check if we were interrupted */
-
-       wasinterrupted = t->interrupted;
-
-       /* reset all flags */
-
-       t->interrupted = false;
-       t->signaled    = false;
        t->sleeping    = false;
 
        /* release the waitmutex */
 
        pthread_mutex_unlock(&t->waitmutex);
-
-       return wasinterrupted;
 }
 
 
@@ -2124,13 +2108,9 @@ static bool threads_wait_with_timeout(threadobject *t, struct timespec *wakeupTi
          millis.......milliseconds to wait
          nanos........nanoseconds to wait
 
-   RETURN VALUE:
-      true.........if the wait has been interrupted,
-         false........if the wait was ended by notification or timeout
-
 *******************************************************************************/
 
-bool threads_wait_with_timeout_relative(threadobject *thread, s8 millis,
+void threads_wait_with_timeout_relative(threadobject *thread, s8 millis,
                                                                                s4 nanos)
 {
        struct timespec wakeupTime;
@@ -2141,7 +2121,7 @@ bool threads_wait_with_timeout_relative(threadobject *thread, s8 millis,
 
        /* wait */
 
-       return threads_wait_with_timeout(thread, &wakeupTime);
+       threads_wait_with_timeout(thread, &wakeupTime);
 }
 
 
@@ -2228,6 +2208,8 @@ bool threads_check_if_interrupted_and_reset(void)
 
        thread = THREADOBJECT;
 
+       pthread_mutex_lock(&thread->waitmutex);
+
        /* get interrupted flag */
 
        intr = thread->interrupted;
@@ -2236,6 +2218,8 @@ bool threads_check_if_interrupted_and_reset(void)
 
        thread->interrupted = false;
 
+       pthread_mutex_unlock(&thread->waitmutex);
+
        return intr;
 }
 
@@ -2274,7 +2258,9 @@ void threads_sleep(s8 millis, s4 nanos)
 
        threads_calc_absolute_time(&wakeupTime, millis, nanos);
 
-       wasinterrupted = threads_wait_with_timeout(thread, &wakeupTime);
+       threads_wait_with_timeout(thread, &wakeupTime);
+
+       wasinterrupted = threads_check_if_interrupted_and_reset();
 
        if (wasinterrupted)
                exceptions_throw_interruptedexception();
index ed523f65b5760cd664dff8bacf1d8781e5540faa..f02f38871e9b8a64c9961927611322fedd6654d3 100644 (file)
@@ -238,7 +238,7 @@ void threads_join_all_threads(void);
 
 void threads_sleep(s8 millis, s4 nanos);
 
-bool threads_wait_with_timeout_relative(threadobject *t, s8 millis, s4 nanos);
+void threads_wait_with_timeout_relative(threadobject *t, s8 millis, s4 nanos);
 
 void threads_thread_interrupt(threadobject *thread);
 bool threads_check_if_interrupted_and_reset(void);