fujimoto/flare — pull request #23 by ikehara (2011-12-27 03:22:50+00:00)
Title: Threads waiting in the thread pool may be tiggered unintentionally under high LA situation.

Flare has a thread pooing mechanism to avoid the overhead of thread creation. When a thread object finishes handler's run() method, it's moved from the global map to the thread pool and waits for the condition variable to be triggered by another requester.

The wait() routine does:
1: lock(mutex);
2: if(!this->_triggered) cond_wait(mutex, condvar);
3: unlock(mutex);

The trigger() routine does:
4: lock(mutex);
5: this->_triggered = true;
6: unlock(mutex);
7: cond_signal(condvar);

If the requester triggers the pooled thread before it enters the critical section of wait(), cond_signal (line 7) does nothing. In case of that situation, trigger() turns the flag '_triggered' on and then sends a signal via the conditional variable. But under high work load, a context switching may occur promptly after the critical section (line 6) and the thread fall through the if statement because the flag is on.

The problem happens when the thread finishes handler's run() and comes back to wait() to be pooled and then the requester gets the control back. The requester hasn't finished trigger() yet and let the initialized thread go by sending a spurious signal. The thread hasn't a thread handler and terminates immediately with a waring message. The thread object is still in the thread pool so a requester which acquires a thread in the future may get a dead thread object that never executes a requested handler.

I think cond_signal (line 7) should be moved into the critical section (line 4-6) to unify these 2 operations.