From 642883f832867bf2fcb70aa702a68ba825c7118d Mon Sep 17 00:00:00 2001 From: sr Date: Wed, 15 Sep 2010 19:25:24 +0200 Subject: [PATCH] * 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). --- src/threads/thread.cpp | 30 ------------------------------ 1 file changed, 30 deletions(-) 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(); } -- 2.25.1