task.cpp:

- Revert the last resort execution of workFunc in Task::Impl::finish(). Windows now has much better compliance with the behavior of pthread_cond_wait(), so the last resort execution is no longer necessary.
This commit is contained in:
rogerman 2016-08-26 10:57:20 +00:00
parent 3ae591be7a
commit 386d9bad96
2 changed files with 98 additions and 50 deletions

View File

@ -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
}
}

View File

@ -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);