* src/threads/thread.cpp (thread_set_state_*): Don't take the ThreadList

lock, most of the time.

We need the lock only when changing the state to terminated. This prevents
other threads from using a stale threadobject * (they have to take the lock,
of course). This changeset removes a nasty deadlock:

Thread 1 locks ThreadList::_mutex -> waitmutex (via thread_handle_interrupt
-> threads_thread_interrupt)
Thread 2 locks waitmutex -> ThreadList::_mutex (via
threads_wait_with_timeout -> thread_set_state_runnable)

The deadlock is provoked by running tests/threads/waitAndInterrupt.java (in
the CACAO source tree). The bug was introduced in changeset a62d7ef60606 (PR
120).

diff --git a/src/threads/thread.cpp b/src/threads/thread.cpp
index bae8161187875bfa5451d69c81704d1892f67db9..7fd0205e2a0efc69aea69e789d8109bc9a745bbe 100644 (file)
--- a/src/threads/thread.cpp
+++ b/src/threads/thread.cpp
@@ -1009,17 +1009,11 @@ static inline void thread_set_state(threadobject *t, int state)
 
 void thread_set_state_runnable(threadobject *t)
 {
-       /* Set the state inside a lock. */
-
-       ThreadList::lock();
-
        if (t->state != THREAD_STATE_TERMINATED) {
                thread_set_state(t, THREAD_STATE_RUNNABLE);
 
                DEBUGTHREADS("is RUNNABLE", t);
        }
-
-       ThreadList::unlock();
 }
 
 
@@ -1034,17 +1028,11 @@ void thread_set_state_runnable(threadobject *t)
 
 void thread_set_state_waiting(threadobject *t)
 {
-       /* Set the state inside a lock. */
-
-       ThreadList::lock();
-
        if (t->state != THREAD_STATE_TERMINATED) {
                thread_set_state(t, THREAD_STATE_WAITING);
 
                DEBUGTHREADS("is WAITING", t);
        }
-
-       ThreadList::unlock();
 }
 
 
@@ -1060,17 +1048,11 @@ void thread_set_state_waiting(threadobject *t)
 
 void thread_set_state_timed_waiting(threadobject *t)
 {
-       /* Set the state inside a lock. */
-
-       ThreadList::lock();
-
        if (t->state != THREAD_STATE_TERMINATED) {
                thread_set_state(t, THREAD_STATE_TIMED_WAITING);
 
                DEBUGTHREADS("is TIMED_WAITING", t);
        }
-
-       ThreadList::unlock();
 }
 
 
@@ -1085,17 +1067,11 @@ void thread_set_state_timed_waiting(threadobject *t)
 
 void thread_set_state_parked(threadobject *t)
 {
-       /* Set the state inside a lock. */
-
-       ThreadList::lock();
-
        if (t->state != THREAD_STATE_TERMINATED) {
                thread_set_state(t, THREAD_STATE_PARKED);
 
                DEBUGTHREADS("is PARKED", t);
        }
-
-       ThreadList::unlock();
 }
 
 
@@ -1110,17 +1086,11 @@ void thread_set_state_parked(threadobject *t)
 
 void thread_set_state_timed_parked(threadobject *t)
 {
-       /* Set the state inside a lock. */
-
-       ThreadList::lock();
-
        if (t->state != THREAD_STATE_TERMINATED) {
                thread_set_state(t, THREAD_STATE_TIMED_PARKED);
 
                DEBUGTHREADS("is TIMED_PARKED", t);
        }
-
-       ThreadList::unlock();
 }