Hi Dean, Thanks for your input.
I think a monitor would be less ideal here: A monitor would introduce blocking in enqueuing operations. The "posters" are (in this case) remotely injected threads (by another process), and the processes injecting those threads are in WaitForSingleObject() on those thread handles. If a monitor is used, it needs to be held by the AttachListener over executing (arbitrary) operations. Granted, each operation today is being executed in serial fashion (but it does not need to be), but I believe we would not like to have each client also block on the monitor as part of enqueuing their operation, just in order to issue the notify. With a Windows Semaphore, clients can act asynchronously in regard to enqueuing. As long as the semaphore's current count is > 0, there is no blocking on the part of the AttachListener thread, WaitForSingleObject() return immediately decrementing the current count. Thanks Markus -----Original Message----- From: Dean Long Sent: den 3 mars 2015 02:32 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) Couldn't you instead treat it like a monitor? Then it's OK if only the first notify wakes up the blocked thread, as long as that thread only blocks when the queue is empty. In other words, when it wakes up, it should process all the items in the queue before blocking again. dl On 3/2/2015 4:34 AM, 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
