RFR: 8257831: Suspend with handshakes [v9]

Robbin Ehn rehn at openjdk.java.net
Thu Apr 15 07:14:14 UTC 2021


On Wed, 14 Apr 2021 16:27:10 GMT, Daniel D. Daugherty <dcubed at openjdk.org> wrote:

>> Robbin Ehn has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 13 commits:
>> 
>>  - Merge branch 'master' into SuspendInHandshake
>>  - Review fixes 4
>>  - Fixed flag undef dep + spelling error
>>  - Obsolete unused flags
>>  - Review fixes 3
>>  - Merge branch 'master' into SuspendInHandshake
>>  - Review fixes 2
>>  - White space fixes
>>  - Merge branch 'master' into SuspendInHandshake
>>  - Review fixes
>>  - ... and 3 more: https://git.openjdk.java.net/jdk/compare/b224b566...27bf041c
>
> src/hotspot/share/runtime/handshake.cpp line 415:
> 
>> 413:   // Adds are done lock free and so is arming.
>> 414:   // Calling this method with lock held is considered an error.
>> 415:   assert(!_lock.owned_by_self(), "Lock should not be held");
> 
> I originally added this comment inside the "resolved" conversation and that kept
> this comment from showing up. Let's try it as a new comment.
> 
> Doesn't that mean the comment on L415 needs updating?
> Should it be something like:
> 
> // Synchronous adds are done lock free and so is arming, but some
> // asynchronous adds are done when we already hold the lock.
> 
> I'm not sure about the "some asynchronous adds" part...
> @dcubed-ojdk

The mutex is unrelated to adds/inserts.
Adds/inserts to the queue can always be done regardless of which locks a thread may or may not have.
The reason for not allowing inserts while holding the HandshakeState lock was to eliminate that from the table, since it have other implications.
As @pchilano found we needed to reverse the order in should_process() to make it work (which I missed).

So now when we have looked at the case, it is okay to do it.

Meaning that you can add a handshake to yourself from another handshake regardless of it is sync or async.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3191


More information about the serviceability-dev mailing list