diff --git a/libretro-common/rthreads/rthreads.c b/libretro-common/rthreads/rthreads.c index 95e32a2c5e..84a66a8b42 100644 --- a/libretro-common/rthreads/rthreads.c +++ b/libretro-common/rthreads/rthreads.c @@ -41,6 +41,7 @@ #define _WIN32_WINNT 0x0500 /*_WIN32_WINNT_WIN2K */ #endif #include +#include #endif #elif defined(GEKKO) #include "gx_pthread.h" @@ -81,16 +82,45 @@ struct sthread struct slock { #ifdef USE_WIN32_THREADS - HANDLE lock; + CRITICAL_SECTION lock; #else pthread_mutex_t lock; #endif }; +#ifdef USE_WIN32_THREADS +/* The syntax we'll use is mind-bending unless we use a struct. Plus, we might want to store more info later */ +/* This will be used as a linked list immplementing a queue of waiting threads */ +struct QueueEntry +{ + struct QueueEntry *next; +}; +#endif + struct scond { #ifdef USE_WIN32_THREADS + /* With this implementation of scond, we don't have any way of waking (or even identifying) specific threads */ + /* But we need to wake them in the order indicated by the queue. */ + /* This potato token will get get passed around every waiter. The bearer can test whether he's next, and hold onto the potato if he is. */ + /* When he's done he can then put it back into play to progress the queue further */ + HANDLE hot_potato; + + /* The primary signalled event. Hot potatoes are passed until this is set. */ HANDLE event; + + /* the head of the queue; NULL if queue is empty */ + struct QueueEntry *head; + + /* equivalent to the queue length */ + int waiters; + + /* how many waiters in the queue have been conceptually wakened by signals (even if we haven't managed to actually wake them yet) */ + int wakens; + + /* used to control access to this scond, in case the user fails */ + CRITICAL_SECTION cs; + #else pthread_cond_t cond; #endif @@ -167,7 +197,7 @@ error: * @thread : pointer to thread object * * Detach a thread. When a detached thread terminates, its - * resource sare automatically released back to the system + * resources are automatically released back to the system * without the need for another thread to join with the * terminated thread. * @@ -214,6 +244,9 @@ void sthread_join(sthread_t *thread) */ bool sthread_isself(sthread_t *thread) { + /* This thread can't possibly be a null thread */ + if (!thread) return false; + #ifdef USE_WIN32_THREADS return GetCurrentThread() == thread->thread; #else @@ -231,23 +264,25 @@ bool sthread_isself(sthread_t *thread) **/ slock_t *slock_new(void) { + bool mutex_created = false; slock_t *lock = (slock_t*)calloc(1, sizeof(*lock)); if (!lock) return NULL; #ifdef USE_WIN32_THREADS - lock->lock = CreateMutex(NULL, FALSE, NULL); - if (!lock->lock) - goto error; + InitializeCriticalSection(&lock->lock); + mutex_created = true; #else - if ((pthread_mutex_init(&lock->lock, NULL) < 0)) - goto error; + mutex_created = (pthread_mutex_init(&lock->lock, NULL) == 0); #endif + if (!mutex_created) + goto error; + return lock; error: - slock_free(lock); + free(lock); return NULL; } @@ -263,7 +298,7 @@ void slock_free(slock_t *lock) return; #ifdef USE_WIN32_THREADS - CloseHandle(lock->lock); + DeleteCriticalSection(&lock->lock); #else pthread_mutex_destroy(&lock->lock); #endif @@ -283,7 +318,7 @@ void slock_lock(slock_t *lock) if (!lock) return; #ifdef USE_WIN32_THREADS - WaitForSingleObject(lock->lock, INFINITE); + EnterCriticalSection(&lock->lock); #else pthread_mutex_lock(&lock->lock); #endif @@ -300,7 +335,7 @@ void slock_unlock(slock_t *lock) if (!lock) return; #ifdef USE_WIN32_THREADS - ReleaseMutex(lock->lock); + LeaveCriticalSection(&lock->lock); #else pthread_mutex_unlock(&lock->lock); #endif @@ -317,21 +352,40 @@ void slock_unlock(slock_t *lock) **/ scond_t *scond_new(void) { - bool event_created = false; scond_t *cond = (scond_t*)calloc(1, sizeof(*cond)); if (!cond) return NULL; #ifdef USE_WIN32_THREADS - cond->event = CreateEvent(NULL, FALSE, FALSE, NULL); - event_created = !!cond->event; -#else - event_created = (pthread_cond_init(&cond->cond, NULL) == 0); -#endif - if (!event_created) + /* This is very complex because recreating condition variable semantics with win32 parts is not easy */ + /* The main problem is that a condition variable can't be used to "pre-wake" a thread (it will get wakened only after it's waited) */ + /* whereas a win32 event can pre-wake a thread (the event will be set in advance, so a 'waiter' won't even have to wait on it) */ + /* Keep in mind a condition variable can apparently pre-wake a thread, insofar as spurious wakeups are always possible, */ + /* but nobody will be expecting this and it does not need to be simulated. */ + /* Moreover, we won't be doing this, because it counts as a spurious wakeup -- someone else with a genuine claim must get wakened, in any case. */ + /* Therefore we choose to wake only one of the correct waiting threads. */ + /* So at the very least, we need to do something clever. But there's bigger problems. */ + /* We don't even have a straightforward way in win32 to satisfy pthread_cond_wait's atomicity requirement. The bulk of this algorithm is solving that. */ + /* Note: We might could simplify this using vista+ condition variables, but we wanted an XP compatible solution. */ + cond->event = CreateEvent(NULL, FALSE, FALSE, NULL); + if (!cond->event) goto error; + cond->hot_potato = CreateEvent(NULL, FALSE, FALSE, NULL); + if (!cond->hot_potato) + { + CloseHandle(cond->event); goto error; + } + + InitializeCriticalSection(&cond->cs); + cond->waiters = cond->wakens = 0; + cond->head = NULL; + +#else + if(pthread_cond_init(&cond->cond, NULL) != 0) + goto error; +#endif return cond; @@ -353,12 +407,163 @@ void scond_free(scond_t *cond) #ifdef USE_WIN32_THREADS CloseHandle(cond->event); + CloseHandle(cond->hot_potato); + DeleteCriticalSection(&cond->cs); #else pthread_cond_destroy(&cond->cond); #endif free(cond); } +#ifdef USE_WIN32_THREADS +static bool _scond_wait_win32(scond_t *cond, slock_t *lock, DWORD dwMilliseconds) +{ + static bool beginPeriod = false; + + struct QueueEntry myentry; + struct QueueEntry **ptr; + DWORD tsBegin; + DWORD dwFinalTimeout = dwMilliseconds; /* Careful! in case we begin in the head, we don't do the hot potato stuff, so this timeout needs presetting */ + DWORD waitResult; + + /* Reminder: `lock` is held before this is called. */ + /* however, someone else may have called scond_signal without the lock. soo... */ + EnterCriticalSection(&cond->cs); + + /* since this library is meant for realtime game software I have no problem setting this to 1 and forgetting about it. */ + if(!beginPeriod) + { + beginPeriod = true; + timeBeginPeriod(1); + } + + /* Now we can take a good timestamp for use in faking the timeout ourselves. */ + /* But don't bother unless we need to (to save a little time) */ + if(dwMilliseconds != INFINITE) + tsBegin = timeGetTime(); + + /* add ourselves to a queue of waiting threads */ + ptr = &cond->head; + while(*ptr) /* walk to the end of the linked list */ + ptr = &((*ptr)->next); + *ptr = &myentry; + myentry.next = NULL; + + cond->waiters++; + + /* 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 */ + + /* It's my turn if I'm the head of the queue. Check to see if it's my turn. */ + while (cond->head != &myentry) + { + /* It isn't my turn: */ + + DWORD timeout = INFINITE; + + /* As long as someone is even going to be able to wake up when they receive the potato, keep it going round */ + if (cond->wakens > 0) + SetEvent(cond->hot_potato); + + /* Assess the remaining timeout time */ + if(dwMilliseconds != INFINITE) + { + DWORD now = timeGetTime(); + DWORD elapsed = now - tsBegin; + if(elapsed > dwMilliseconds) + { + /* Try one last time with a zero timeout (keeps the code simpler) */ + elapsed = dwMilliseconds; + } + timeout = dwMilliseconds - elapsed; + } + + /* Let someone else go */ + LeaveCriticalSection(&lock->lock); + LeaveCriticalSection(&cond->cs); + + /* Wait a while to catch the hot potato.. someone else should get a chance to go */ + /* After all, it isn't my turn (and it must be someone else's) */ + Sleep(0); + waitResult = WaitForSingleObject(cond->hot_potato, timeout); + + /* I should come out of here with the main lock taken */ + EnterCriticalSection(&lock->lock); + EnterCriticalSection(&cond->cs); + + if(waitResult == WAIT_TIMEOUT) + { + /* Out of time! Now, let's think about this. I do have the potato now--maybe it's my turn, and I have the event? */ + /* If that's the case, I could proceed right now without aborting due to timeout. */ + /* However.. I DID wait a real long time. The caller was willing to wait that long. */ + /* I choose to give him one last chance with a zero timeout in the next step */ + if(cond->head == &myentry) + { + dwFinalTimeout = 0; + break; + } + else + { + /* It's not our turn and we're out of time. Give up. */ + /* Remove ourself from the queue and bail. */ + struct QueueEntry* curr = cond->head; + while(curr->next != &myentry) curr = curr->next; + curr->next = myentry.next; + cond->waiters--; + LeaveCriticalSection(&cond->cs); + return false; + } + } + + } + + /* It's my turn now -- and I hold the potato */ + + /* I still have the main lock, in any case */ + /* I need to release it so that someone can set the event */ + LeaveCriticalSection(&lock->lock); + LeaveCriticalSection(&cond->cs); + + /* Wait for someone to actually signal this condition */ + /* We're the only waiter waiting on the event right now -- everyone else is waiting on something different */ + waitResult = WaitForSingleObject(cond->event, dwFinalTimeout); + + /* Take the main lock so we can do work. Nobody else waits on this lock for very long, so even though it's GO TIME we won't have to wait long */ + EnterCriticalSection(&lock->lock); + EnterCriticalSection(&cond->cs); + + /* Remove ourselves from the queue */ + cond->head = myentry.next; + cond->waiters--; + + if(waitResult == WAIT_TIMEOUT) + { + /* Oops! ran out of time in the final wait. Just bail. */ + LeaveCriticalSection(&cond->cs); + return false; + } + + /* If any other wakenings are pending, go ahead and set it up */ + /* There may actually be no waiters. That's OK. The first waiter will come in, find it's his turn, and immediately get the signaled event */ + cond->wakens--; + if (cond->wakens > 0) + { + SetEvent(cond->event); + + /* Progress the queue: Put the hot potato back into play. It'll be tossed around until next in line gets it */ + SetEvent(cond->hot_potato); + } + + LeaveCriticalSection(&cond->cs); + return true; +} +#endif + /** * scond_wait: * @cond : pointer to condition variable object @@ -369,10 +574,7 @@ void scond_free(scond_t *cond) void scond_wait(scond_t *cond, slock_t *lock) { #ifdef USE_WIN32_THREADS - WaitForSingleObject(cond->event, 0); - - SignalObjectAndWait(lock->lock, cond->event, INFINITE, FALSE); - slock_lock(lock); + _scond_wait_win32(cond, lock, INFINITE); #else pthread_cond_wait(&cond->cond, &lock->lock); #endif @@ -388,9 +590,17 @@ 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->waiters == 0) return 0; + + /* awaken everything which is currently queued up */ + if(cond->wakens == 0) SetEvent(cond->event); + cond->wakens = cond->waiters; + + /* Since there is now at least one pending waken, the potato must be in play */ + SetEvent(cond->hot_potato); + return 0; #else return pthread_cond_broadcast(&cond->cond); @@ -407,7 +617,33 @@ int scond_broadcast(scond_t *cond) void scond_signal(scond_t *cond) { #ifdef USE_WIN32_THREADS - SetEvent(cond->event); + + /* Unfortunately, pthread_cond_signal does not require that the lock be held in advance */ + /* To avoid stomping on the condvar from other threads, we need to control access to it with this */ + EnterCriticalSection(&cond->cs); + + /* remember: we currently have mutex */ + if(cond->waiters == 0) + { + LeaveCriticalSection(&cond->cs); + return; + } + + /* wake up the next thing in the queue */ + if(cond->wakens == 0) SetEvent(cond->event); + cond->wakens++; + + /* The data structure is done being modified.. I think we can leave the CS now. */ + /* This would prevent some other thread from receiving the hot potato and then + /* immediately stalling for the critical section. */ + /* But remember, we were trying to replicate a semantic where this entire scond_signal call + /* was controlled (by the user) by a lock. */ + /* So in case there's trouble with this, we can move it after SetEvent() */ + LeaveCriticalSection(&cond->cs); + + /* Since there is now at least one pending waken, the potato must be in play */ + SetEvent(cond->hot_potato); + #else pthread_cond_signal(&cond->cond); #endif @@ -428,14 +664,20 @@ void scond_signal(scond_t *cond) bool scond_wait_timeout(scond_t *cond, slock_t *lock, int64_t timeout_us) { #ifdef USE_WIN32_THREADS - DWORD ret; - - WaitForSingleObject(cond->event, 0); - ret = SignalObjectAndWait(lock->lock, cond->event, - (DWORD)(timeout_us) / 1000, FALSE); - - slock_lock(lock); - return ret == WAIT_OBJECT_0; + /* How to convert a us timeout to ms? */ + /* Someone asking for a 0 timeout clearly wants immediate timeout. */ + /* Someone asking for a 1 timeout clearly wants an actual timeout of the minimum length */ + /* Someone asking for 1000 or 1001 timeout shouldn't accidentally get 2ms. */ + DWORD dwMilliseconds = timeout_us/1000; + if(timeout_us == 0) { + /* The implementation of a 0 timeout here with pthreads is sketchy. */ + /* It isn't clear what happens if pthread_cond_timedwait is called with NOW. */ + /* Moreover, it is possible that this thread gets pre-empted after the clock_gettime but before the pthread_cond_timedwait. */ + /* In order to help smoke out problems caused by this strange usage, let's treat a 0 timeout as always timing out. */ + return false; + } + else if(timeout_us < 1000) dwMilliseconds = 1; + return _scond_wait_win32(cond,lock,dwMilliseconds); #else int ret; int64_t seconds, remainder;