From: sr <none@none>
Date: Wed, 15 Sep 2010 17:25:24 +0000 (+0200)
Subject: * src/threads/thread.cpp (thread_set_state_*): Don't take the ThreadList
X-Git-Url: http://wien.tomnetworks.com/gitweb/?p=cacao.git;a=commitdiff_plain;h=642883f832867bf2fcb70aa702a68ba825c7118d

* 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 bae816118..7fd0205e2 100644
--- 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();
 }