diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c index 706f5bb3b..8b86617ff 100644 --- a/src/backend/threadpool.c +++ b/src/backend/threadpool.c @@ -57,12 +57,10 @@ lumiera_threadpool_init() //TODO: configure each pools' pthread_attrs appropriately pthread_attr_init (&threadpool.pool[i].pthread_attrs); - // cehteh prefers that threads are joinable by default - //pthread_attr_setdetachstate (&threadpool.pool[i].pthread_attrs, PTHREAD_CREATE_DETACHED); + // cehteh says that threads must be joinable //cancel... - lumiera_mutex_init(&threadpool.pool[i].lock,"pool of threads", &NOBUG_FLAG(threadpool)); - lumiera_reccondition_init (&threadpool.pool[i].signal, "thread-signal", &NOBUG_FLAG(threadpool)); + lumiera_condition_init (&threadpool.pool[i].sync, "threadpool", &NOBUG_FLAG(cond_sync)); } } @@ -74,7 +72,7 @@ lumiera_threadpool_destroy(void) for (int i = 0; i < LUMIERA_THREADCLASS_COUNT; ++i) { TRACE (threadpool, "destroying individual pool #%d", i); - LUMIERA_MUTEX_SECTION (threadpool, &threadpool.pool[i].lock) + LUMIERA_CONDITION_SECTION (threadpool, &threadpool.pool[i].sync) { REQUIRE (0 == threadpool.pool[i].working_thread_count, "%d threads are running", threadpool.pool[i].working_thread_count); // TODO need to have a stronger assertion that no threads are really running because they will not even be in the list @@ -84,9 +82,8 @@ lumiera_threadpool_destroy(void) lumiera_thread_delete((LumieraThread)t); } } - ECHO ("destroying the pool mutex"); - lumiera_mutex_destroy (&threadpool.pool[i].lock, &NOBUG_FLAG (threadpool)); - ECHO ("pool mutex destroyed"); + + lumiera_condition_destroy (&threadpool.pool[i].sync, &NOBUG_FLAG (cond_sync)); pthread_attr_destroy (&threadpool.pool[i].pthread_attrs); } } @@ -100,16 +97,17 @@ lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, TRACE(threadpool); LumieraThread ret = NULL; REQUIRE (kind < LUMIERA_THREADCLASS_COUNT, "unknown pool kind specified: %d", kind); - LUMIERA_RECCONDITION_SECTION (threadpool, &threadpool.pool[kind].signal) - { - if (llist_is_empty (&threadpool.pool[kind].list)) - { - ret = lumiera_thread_new (kind, purpose, flag, - &threadpool.pool[kind].pthread_attrs); - threadpool.pool[kind].idle_thread_count++; - ENSURE (ret, "did not create a valid thread"); - LUMIERA_RECCONDITION_WAIT (!llist_is_empty (&threadpool.pool[kind].list)); + LUMIERA_CONDITION_SECTION (threadpool, &threadpool.pool[kind].sync) + { + if (llist_is_empty (&threadpool.pool[kind].list)) + { + + ret = lumiera_thread_new (kind, purpose, flag, + &threadpool.pool[kind].pthread_attrs); + ENSURE (ret, "did not create a valid thread"); + TODO("no error handling but let the resourcecollector do, no need for return the thread"); + LUMIERA_CONDITION_WAIT (!llist_is_empty (&threadpool.pool[kind].list)); } // use an existing thread, pick the first one // remove it from the pool's list @@ -129,7 +127,7 @@ lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, // TODO: rename to lumiera_threadpool_park_thread void -lumiera_threadpool_park_thread(LumieraThread thread) +lumiera_threadpool_release_thread(LumieraThread thread) { TRACE(threadpool); REQUIRE (thread, "invalid thread given"); @@ -137,20 +135,22 @@ lumiera_threadpool_park_thread(LumieraThread thread) REQUIRE (thread->state != LUMIERA_THREADSTATE_IDLE, "trying to park an already idle thread"); - LUMIERA_RECCONDITION_SECTION (threadpool, &threadpool.pool[thread->kind].signal) + LUMIERA_CONDITION_SECTION (threadpool, &threadpool.pool[thread->kind].sync) { thread->state = LUMIERA_THREADSTATE_IDLE; - REQUIRE (llist_is_single(&thread->node), "thread already belongs to some list"); + REQUIRE (!llist_is_empty(&thread->node), "thread already belongs to some list"); llist_insert_head(&threadpool.pool[thread->kind].list, &thread->node); - threadpool.pool[thread->kind].working_thread_count--; - threadpool.pool[thread->kind].idle_thread_count++; // cheaper than using llist_count - ENSURE (threadpool.pool[thread->kind].idle_thread_count == - llist_count(&threadpool.pool[thread->kind].list), - "idle thread count %d is wrong, should be %d", - threadpool.pool[thread->kind].idle_thread_count, - llist_count(&threadpool.pool[thread->kind].list)); - // REQUIRE (!llist_is_empty (&threadpool.pool[thread->kind].list), "thread pool is still empty after insertion"); - LUMIERA_RECCONDITION_BROADCAST; + + threadpool.pool[thread->kind].working_thread_count--; + threadpool.pool[thread->kind].idle_thread_count++; // cheaper than using llist_count + + ENSURE (threadpool.pool[thread->kind].idle_thread_count == + llist_count(&threadpool.pool[thread->kind].list), + "idle thread count %d is wrong, should be %d", + threadpool.pool[thread->kind].idle_thread_count, + llist_count(&threadpool.pool[thread->kind].list)); + REQUIRE (!llist_is_empty (&threadpool.pool[thread->kind].list), "thread pool is still empty after insertion"); + LUMIERA_CONDITION_BROADCAST; } } diff --git a/src/backend/threadpool.h b/src/backend/threadpool.h index 572727e3e..a8fa457c5 100644 --- a/src/backend/threadpool.h +++ b/src/backend/threadpool.h @@ -23,9 +23,8 @@ #define LUMIERA_THREADPOOL_H //TODO: Support library includes// -#include "lib/reccondition.h" +#include "lib/condition.h" #include "lib/llist.h" -#include "lib/mutex.h" //TODO: Forward declarations// @@ -60,7 +59,7 @@ lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, * This function doesn't need to be accessible outside of the threadpool implementation. */ void -lumiera_threadpool_park_thread(LumieraThread thread); +lumiera_threadpool_release_thread(LumieraThread thread); typedef struct lumiera_threadpool_struct lumiera_threadpool; typedef lumiera_threadpool* LumieraThreadpool; @@ -70,11 +69,10 @@ struct lumiera_threadpool_struct struct { llist list; - lumiera_mutex lock; unsigned working_thread_count; unsigned idle_thread_count; pthread_attr_t pthread_attrs; - lumiera_reccondition signal; + lumiera_condition sync; } pool[LUMIERA_THREADCLASS_COUNT]; }; diff --git a/src/backend/threads.c b/src/backend/threads.c index a34dcde4a..a7f09b763 100644 --- a/src/backend/threads.c +++ b/src/backend/threads.c @@ -22,7 +22,6 @@ //TODO: Support library includes// #include "include/logging.h" -#include "lib/mutex.h" #include "lib/safeclib.h" @@ -64,7 +63,7 @@ struct lumiera_thread_mockup { void (*fn)(void*); void* arg; - LumieraReccondition finished; + LumieraCondition finished; }; static void* thread_loop (void* thread) @@ -73,21 +72,20 @@ static void* thread_loop (void* thread) LumieraThread t = (LumieraThread)thread; pthread_setcancelstate (PTHREAD_CANCEL_DISABLE, NULL); - + REQUIRE (t, "thread does not exist"); - ECHO ("entering section 1"); // this seems to deadlock unexpectedly: - LUMIERA_RECCONDITION_SECTION (threads, &t->signal) + LUMIERA_CONDITION_SECTION (threads, &t->signal) { do { // NULL function means: no work to do INFO(threads, "function %p", t->function); if (t->function) t->function (t->arguments); + lumiera_threadpool_release_thread(t); + LUMIERA_CONDITION_WAIT(t->state != LUMIERA_THREADSTATE_IDLE); INFO(threads, "Thread awaken with state %d", t->state); - lumiera_threadpool_park_thread(t); - LUMIERA_RECCONDITION_WAIT(t->state != LUMIERA_THREADSTATE_IDLE); } while (t->state != LUMIERA_THREADSTATE_SHUTDOWN); // SHUTDOWN state @@ -117,9 +115,9 @@ lumiera_thread_run (enum lumiera_thread_class kind, // and let it really run (signal the condition var, the thread waits on it) self->state = LUMIERA_THREADSTATE_WAKEUP; - ECHO ("entering section 2"); - LUMIERA_RECCONDITION_SECTION(threads, &self->signal) - LUMIERA_RECCONDITION_BROADCAST; + + LUMIERA_CONDITION_SECTION(threads, &self->signal) + LUMIERA_CONDITION_SIGNAL; // NOTE: example only, add solid error handling! @@ -142,7 +140,7 @@ lumiera_thread_new (enum lumiera_thread_class kind, LumieraThread self = lumiera_malloc (sizeof (*self)); llist_init(&self->node); - lumiera_reccondition_init (&self->signal, "thread-control", flag); + lumiera_condition_init (&self->signal, "thread-control", flag); self->kind = kind; self->state = LUMIERA_THREADSTATE_STARTUP; self->function = NULL; @@ -170,20 +168,20 @@ lumiera_thread_destroy (LumieraThread self) // get the pthread out of the processing loop // need to signal to the thread that it should start quitting // should this be within the section? - LUMIERA_RECCONDITION_SECTION(threads, &self->signal) + LUMIERA_CONDITION_SECTION(threads, &self->signal) { REQUIRE (self->state == LUMIERA_THREADSTATE_IDLE, "trying to delete a thread in state other than IDLE (%s)", lumiera_threadstate_names[self->state]); self->state = LUMIERA_THREADSTATE_SHUTDOWN; self->function = NULL; self->arguments = NULL; - LUMIERA_RECCONDITION_SIGNAL; + LUMIERA_CONDITION_SIGNAL; } int error = pthread_join(self->id, NULL); ENSURE (0 == error, "pthread_join returned %d:%s", error, strerror(error)); // condition has to be destroyed after joining with the thread - lumiera_reccondition_destroy (&self->signal, &NOBUG_FLAG(threads)); + lumiera_condition_destroy (&self->signal, &NOBUG_FLAG(threads)); return self; } diff --git a/src/backend/threads.h b/src/backend/threads.h index 9a4e6bbdf..d4282da17 100644 --- a/src/backend/threads.h +++ b/src/backend/threads.h @@ -23,7 +23,7 @@ #define LUMIERA_THREADS_H //TODO: Support library includes// -#include "lib/reccondition.h" +#include "lib/condition.h" //TODO: Forward declarations// @@ -132,7 +132,7 @@ struct lumiera_thread_struct // void* arg; pthread_t id; // TODO: maybe this condition variable should be renamed when we have a better understanding of how it will be used - lumiera_reccondition signal; // control signal, state change signal + lumiera_condition signal; // control signal, state change signal // the following member could have been called "class" except that it would conflict with C++ keyword // as consequence, it's been decided to leave the type name containing the word "class", // while all members/variables called "kind" diff --git a/tests/backend/test-threads.c b/tests/backend/test-threads.c index 21786ae8b..ddf43214e 100644 --- a/tests/backend/test-threads.c +++ b/tests/backend/test-threads.c @@ -47,15 +47,15 @@ void threadfn(void* blah) void threadsyncfn(void* blah) { struct timespec wait = {0,200000000}; - LumieraReccondition sync = (LumieraReccondition) blah; + LumieraCondition sync = (LumieraCondition) blah; ECHO ("thread starting up %s", NOBUG_THREAD_ID_GET); - LUMIERA_RECCONDITION_SECTION(cond_sync, sync) + LUMIERA_CONDITION_SECTION(cond_sync, sync) { ECHO ("send startup signal %s", NOBUG_THREAD_ID_GET); - LUMIERA_RECCONDITION_SIGNAL; + LUMIERA_CONDITION_SIGNAL; ECHO ("wait for trigger %s", NOBUG_THREAD_ID_GET); - LUMIERA_RECCONDITION_WAIT(1); + LUMIERA_CONDITION_WAIT(1); } ECHO ("thread running %s", NOBUG_THREAD_ID_GET); @@ -101,10 +101,10 @@ TEST ("simple_thread") TEST ("thread_synced") { - lumiera_reccondition cnd; - lumiera_reccondition_init (&cnd, "threadsync", &NOBUG_FLAG(NOBUG_ON)); + lumiera_condition cnd; + lumiera_condition_init (&cnd, "threadsync", &NOBUG_FLAG(NOBUG_ON)); - LUMIERA_RECCONDITION_SECTION(cond_sync, &cnd) + LUMIERA_CONDITION_SECTION(cond_sync, &cnd) { ECHO ("main before thread %s", NOBUG_THREAD_ID_GET); @@ -115,17 +115,17 @@ TEST ("thread_synced") NULL); ECHO ("main wait for thread being ready %s", NOBUG_THREAD_ID_GET); - LUMIERA_RECCONDITION_WAIT(1); + LUMIERA_CONDITION_WAIT(1); ECHO ("main trigger thread %s", NOBUG_THREAD_ID_GET); - LUMIERA_RECCONDITION_SIGNAL; + LUMIERA_CONDITION_SIGNAL; ECHO ("wait for thread end %s", NOBUG_THREAD_ID_GET); - LUMIERA_RECCONDITION_WAIT(1); + LUMIERA_CONDITION_WAIT(1); ECHO ("thread ended %s", NOBUG_THREAD_ID_GET); } - lumiera_reccondition_destroy (&cnd, &NOBUG_FLAG(NOBUG_ON)); + lumiera_condition_destroy (&cnd, &NOBUG_FLAG(NOBUG_ON)); }