Hi Markus,
On 2.3.2015 13:34, Markus Gronlund wrote:
Greetings,
Kindly asking for reviews for the following changeset:
Bug: https://bugs.openjdk.java.net/browse/JDK-8073042
Webrev: http://cr.openjdk.java.net/~mgronlun/8073042/webrev01/
Looks reasonable.
Description:
The signaling mechanism used to communicate about attaching operations
under Windows currently only allows for a single outstanding item to be
visible. This leads to issues, such as the one described in this bug,
where clients assume their operations are under processing (they have
been enqueued after all), but the AttachListener thread does not see,
and hence do not process, these operations.
Analysis:
The _wakeup semaphore only allows for a single outstanding operation:
CreateSemaphore(NULL, 0, 1, NULL);
When a thread has enqueued an operation, it will notify the
AttachListener thread through the semaphore, by:
::ReleaseSemaphore(wakeup(), 1, NULL); // this increases the semaphore
count by 1
This will signal the semaphore and "wakeup" the AttachListener thread
which (most likely) is in WaitForSingleObject() for the semaphore to
become signaled. When the semaphore is signaled and AttachListener
returns from WaitForSingleObject(), the semaphore's count is
decremented, and the semaphore again becomes non-signaled (goes from
current count 1 (which is also maximum count) to zero).
Problem: multiple client threads will enqueue their operations if they
manage to get the list mutex() - if they do, they will insert their op
into the queue, and call:
::ReleaseSemaphore(wakeup(), 1, NULL);
This means we could have two (or more) client threads having posted
operations, before the AttachListener thread becomes scheduled.
Since the semaphore created has a maximum count == 1, any subsequent
calls to ::ReleaseSemaphore(wakeup(), 1, NULL);, taking the the current
count to > maximum count, will have _no_effect - the current count of
the semaphore remains at maximum count level AND
::ReleaseSemaphore(wakeup(), 1, NULL); returns FALSE - but currently
there is no checking the return value here...
This means the client thread has managed to enqueue its op (and will
move into ConnectPipe()), but the AttachListener will never see more
than 1 enqueued op at any one time (hence it does not know it is
expected to process another operation and signal the associated pipe).
This is how operations manage to get enqueued, but not processed until
another thread eventually signals the semaphore by posting another op.
We must allow the semaphore to stay signaled when multiple ops are
enqueued - and since we only allow preallocate_count number of
operations to be enqueued, we can ensure the semaphore manages this
upper limit as its maximum count.
So, if the number of enqueue request is higher than the max semaphore
count it will just fail with the assert?
Thanks,
-JB-
Tests:
Jcmd/dcmd test
Attach tests
Thanks
Markus
