ntdll: Use condition variable for RtlQueueWorkItem implementation

[Sebastian Lackner]

This patch replaces the current ntdll.RtlQueueWorkItem implementation
with a "better" one. While taking a look at the previous
RtlQueueWorkItem implementation I discovered the following problems:

* After adding a new element to the queue NtReleaseSemaphore() is
called, but NtWaitForSingleObject() is called only when the queue is
empty. If a lot of new elements are processed, and threads never wait,
then the counter could (theoretically) overflow.

* Race condition 1: An existing worker thread is just about to
terminate, then a thread switch occurs (before num_workers has been
decremented). A new thread increases the semaphore, but this doesn't
have any effect (no thread waiting on semaphore), and no new thread is
spawned -> the new work will not be processed at all.

* Race condition 2: If it fails to create a new thread and no other
workers are available, the current code tries to remove the work item
again. It is possible that in the meantime a new thread spawns,
which processes the work item. This leads to a duplicate free
(although its unlikely to happen at all).


To solve the problems above I decided to rewrite the complete
RtlQueueWorkItem() implementation by using some of the new condition
variable concepts that recently got upstream. ;-) My changes include:

* Use RtlWakeConditionVariable() instead of events to wake up threads.

* The first race condition (when terminating) is solved by protecting
num_workers with the critical section (instead of using interlocked
commands).

* The second race condition is solved with an additional check, and
removing the element only, when no other workers have spawned in the
meantime.


Some design decisions:

* It is on purpose that RtlWakeConditionVariable is called after
RtlLeaveCriticalSection - this introduces a new "race condition" (wake
up occurs after the item was already processed), but this is harmless.
By swapping the order the race condition wouldn't occur, but it slows
down a bit more, and I think speed is here a bit more important than some
wasted CPU cycles.

* Its also on purpose, that num_workers is (read-) accessed at one point
without locking the critical section. If num_workers > 0 we can be sure, that
a worker thread will process the work item (sooner or later). If
num_workers == 0 we enter the critical section, and check once more to
prevent race condition 2.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ntdll-Use-condition-variable-for-RtlQueueWorkItem-im.patch
Type: text/x-patch
Size: 7229 bytes
Desc: not available
URL: <http://www.winehq.org/pipermail/wine-patches/attachments/20140310/f44990c3/attachment.bin>