diff --git a/desmume/src/libretro-common/rthreads/rthreads.c b/desmume/src/libretro-common/rthreads/rthreads.c index 097a58927..794b09909 100644 --- a/desmume/src/libretro-common/rthreads/rthreads.c +++ b/desmume/src/libretro-common/rthreads/rthreads.c @@ -77,10 +77,21 @@ struct slock #endif }; +#ifdef USE_WIN32_THREADS +//TODO - there's actually no need for this struct. we could do it all with ugly pointer syntax. save that for later. +struct ConceptualBlock +{ + struct ConceptualBlock* next; +}; +#endif + struct scond { #ifdef USE_WIN32_THREADS - HANDLE event; + HANDLE event, hot_potato; + volatile struct ConceptualBlock* volatile root; //the root of the queue; NULL if queue is empty + volatile int waiters; //equivalent to the queue length + volatile int wakens; #else pthread_cond_t cond; #endif @@ -310,7 +321,14 @@ scond_t *scond_new(void) return NULL; #ifdef USE_WIN32_THREADS - cond->event = CreateEvent(NULL, FALSE, FALSE, NULL); + /* this is very complex because recreating condition variable semantics with win32 parts is not easy (or maybe it is and I just havent seen how) */ + /* the main problem is that a condition variable can be used to wake up a thread, but only if the thread is already waiting. */ + /* whereas a win32 event will 'wake up' a thread in advance (the event will be set in advance, so a 'waiter' wont even have to wait on it) */ + cond->event = CreateEvent(NULL, FALSE, FALSE, NULL); + cond->hot_potato = CreateEvent(NULL, FALSE, FALSE, NULL); + cond->waiters = 0; + cond->wakens = 0; + cond->root = NULL; event_created = !!cond->event; #else event_created = (pthread_cond_init(&cond->cond, NULL) == 0); @@ -339,6 +357,7 @@ void scond_free(scond_t *cond) #ifdef USE_WIN32_THREADS CloseHandle(cond->event); + CloseHandle(cond->hot_potato); #else pthread_cond_destroy(&cond->cond); #endif @@ -355,9 +374,66 @@ void scond_free(scond_t *cond) void scond_wait(scond_t *cond, slock_t *lock) { #ifdef USE_WIN32_THREADS - slock_unlock(lock); - WaitForSingleObject(cond->event, INFINITE); - slock_lock(lock); + + //setup a queue (linked list) of blocked threads + volatile struct ConceptualBlock myblock; + myblock.next = NULL; + volatile struct ConceptualBlock* volatile * ptr = &cond->root; + while(*ptr != NULL) + ptr = &((*ptr)->next); + *ptr = &myblock; + + //now the conceptual lock release and condition block are supposed to be atomic. + //we can't do that in windows, but we can simulate the effects by using the queue, by the following analysis: + //What happens if they aren't atomic? + //1. a signaller can rush in and signal, expecting a waiter to get it; but the waiter wouldn't, because he isn't blocked yet + //solution: win32 events make this easy. the event will sit there enabled + //2. a signaller can rush in and signal, and then turn right around and wait + //solution: the signaller will get queued behind the waiter, who's enqueued before he releases the mutex + + for(;;) + { + bool myturn = (cond->root == &myblock); + + if(!myturn) + { + //well, someone else needs to get to go, maybe it's their turn + //NOTE: this depends on good fair behavour of thread blocking in the OS. I think it's OK + SetEvent(cond->hot_potato); + } + + ReleaseMutex(lock->lock); + + if(myturn) + { + WaitForSingleObject(cond->event, INFINITE); + break; + } + else + { + WaitForSingleObject(cond->hot_potato, INFINITE); + + //re-acquire mutex just for interrogating the queue + WaitForSingleObject(lock->lock, INFINITE); + } + } + + //re-acquire mutex + WaitForSingleObject(lock->lock, INFINITE); + + //remove ourselves from the queue + cond->root = myblock.next; + + //if we have any more wakening to do, chain it here + cond->wakens--; + if(cond->wakens>0) + SetEvent(cond->event); + + cond->waiters--; + + //always leave this set. because--TBD: explain later + SetEvent(cond->hot_potato); + #else pthread_cond_wait(&cond->cond, &lock->lock); #endif @@ -373,9 +449,14 @@ void scond_wait(scond_t *cond, slock_t *lock) int scond_broadcast(scond_t *cond) { #ifdef USE_WIN32_THREADS - /* FIXME _- check how this function should differ - * from scond_signal implementation. */ - SetEvent(cond->event); + + /* remember: we currently have mutex */ + if(cond->root == NULL) return 0; + + //awaken everything which is currently queued up + if(cond->wakens == 0) SetEvent(cond->event); + cond->wakens = cond->waiters; + return 0; #else return pthread_cond_broadcast(&cond->cond); @@ -392,7 +473,14 @@ int scond_broadcast(scond_t *cond) void scond_signal(scond_t *cond) { #ifdef USE_WIN32_THREADS - SetEvent(cond->event); + + /* remember: we currently have mutex */ + if(cond->root == NULL) return; + + //wake up the next thing in the queue + if(cond->wakens == 0) SetEvent(cond->event); + cond->wakens++; + #else pthread_cond_signal(&cond->cond); #endif @@ -465,4 +553,4 @@ bool scond_wait_timeout(scond_t *cond, slock_t *lock, int64_t timeout_us) ret = pthread_cond_timedwait(&cond->cond, &lock->lock, &now); return (ret == 0); #endif -} +} \ No newline at end of file diff --git a/desmume/src/utils/task.cpp b/desmume/src/utils/task.cpp index eed37c137..da860e9d6 100644 --- a/desmume/src/utils/task.cpp +++ b/desmume/src/utils/task.cpp @@ -71,7 +71,6 @@ public: void *workFuncParam; void *ret; bool exitThread; - bool isTaskWaiting; }; static void taskProc(void *arg) @@ -82,9 +81,7 @@ static void taskProc(void *arg) slock_lock(ctx->mutex); while (ctx->workFunc == NULL && !ctx->exitThread) { - ctx->isTaskWaiting = true; scond_wait(ctx->condWork, ctx->mutex); - ctx->isTaskWaiting = false; } if (ctx->workFunc != NULL) { @@ -104,7 +101,6 @@ static void taskProc(void *arg) Task::Impl::Impl() { _isThreadRunning = false; - isTaskWaiting = false; workFunc = NULL; workFuncParam = NULL; ret = NULL; @@ -134,7 +130,6 @@ void Task::Impl::start(bool spinlock) this->workFuncParam = NULL; this->ret = NULL; this->exitThread = false; - this->isTaskWaiting = false; this->_thread = sthread_create(&taskProc,this); this->_isThreadRunning = true; @@ -169,41 +164,6 @@ void* Task::Impl::finish() return returnValue; } - // As a last resort, we need to ensure that taskProc() actually executed, and if - // it didn't, do something about it right now. - // - // Normally, calling execute() will wake up taskProc(), but on certain systems, - // the signal from execute() might get missed by taskProc(). If this signal is - // missed, then this method's scond_wait() will hang, since taskProc() will never - // clear workFunc and signal back when its finished (taskProc() was never woken - // up in the first place). - // - // This situation is only possible on systems where scond_wait() does not have - // immediate lock/unlock mechanics with the wait state, such as on Windows. - // Signals can get lost in scond_wait() since a thread's wait state might start - // at a much later time from releasing the mutex, causing the signalling thread - // to send its signal before the wait state is set. All of this is possible - // because of the fact that switching the wait state and switching the mutex - // state are performed as two separate operations. In common parlance, this is - // known as the "lost wakeup problem". - // - // On systems that do have immediate lock/unlock mechanics with the wait state, - // such as systems that natively support pthread_cond_wait(), it is impossible - // for this situation to occur since both the thread wait state and the mutex - // state will switch simultaneously, thus never missing a signal due to the - // constant protection of the mutex. -#if defined(WIN32) - if (this->isTaskWaiting) - { - // In the event where the signal was missed by taskProc(), just do the work - // right now in this thread. Hopefully, signal misses don't happen to often, - // because if they do, it would completely defeat the purpose of having the - // extra task thread in the first place. - this->ret = this->workFunc(workFuncParam); - this->workFunc = NULL; - } -#endif - while (this->workFunc != NULL) { scond_wait(this->condWork, this->mutex);