Hi Dmitry, Thanks for taking a look.
Also thanks for the hint about using a guarantee instead of an assert - you are right, I will change it accordingly. Cheers Markus -----Original Message----- From: Dmitry Samersoff Sent: den 2 mars 2015 14:52 To: Markus Gronlund; serviceability-dev@openjdk.java.net; hotspot-runtime-...@openjdk.java.net Subject: Re: RFR(S): 8073042: jcmd hangs until another jcmd is executed (which, in turn, also hangs) (on Windows) Markus, Looks good for me. Excellent finding. Variable 229 BOOL not_exceeding_semaphore_maximum_count = is not used in production and it could lead to a compiler warning. So it might be better to use guarantee instead of assert here. -Dmitry On 2015-03-02 15: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/ > > > > 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. > > > > Tests: > > > > Jcmd/dcmd test > > Attach tests > > > > Thanks > > Markus > -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
