RFR(s): 8228758: assert(_no_handle_mark_nesting == 0) failed: allocating handle inside NoHandleMark
Robbin Ehn
robbin.ehn at oracle.com
Fri Aug 30 06:56:05 UTC 2019
Hi David, thanks for having a look!
On 2019-08-30 03:24, David Holmes wrote:
> Hi Robbin,
>
> On 29/08/2019 10:48 pm, Robbin Ehn wrote:
>> Hi all, please review.
>>
>> The JNI_QUICK_ENTRY/JVM_QUICK_ENTRY skips creating the HandleMarkCleaner,
>> instead they have a NoHandleMark. Handshakes can
>> be executed by the JavaThread itself, thus might require a
>> HandleMark. QUICK entries allow polls for safepoint/handshakes, if a
>> handshake is executed and a HandleMark is created we hit this
>> assert.
>>
>> HandleMarkCleaner have previous been optimized for inlining, thus cheap.
>> Performance benchmarks shows no benefit of not allowing HandleMark.
>>
>> This changeset removes the the QUICK entry and avoids this issue.
>>
>> Note that JNI_QUICK_ENTRY also don't have WeakPreserveExceptionMark,
>> so it's changed to JNI_ENTRY_NO_PRESERVE.
>
> Why did you extend this to the JNI_QUICK_ENTRY case when the impacted code was
> only a JVM_QUICK_ENTRY? The expanded scope of the change seems unnecessary given
> this is just a workaround for a particular failure mode.
- If your code require you not to use any HandleMarks it most likely require no
polls, for correctness. JRT_LEAF does this correctly, sets up both
NoSafepointVerifier and NoHandleMark. (with the HandleMark in handshake we now
always require NoSafepointVerifier if you use NoHandleMark for correctness)
- There is no performance benefit, so we just have extra code which is useless.
- It might be affected, I have not track down every path. But even if it's
not, nothing is stopping someone from accidentally changing the code to call
something which does a poll, since we don't have NoSafepointVerifier.
So we avoid getting the same failure mode in future.
- Reducing the complexity of interafceSupport is always good!
- There was no reason to keep it.
So if you don't a very strong opinion about it, I really like to get rid of it.
Thanks, Robbin
Some extra words about NoHandleMark:
I believe NoHandleMark should be removed.
In the LEAF case, there is no point in creating a Handle within a
NoSafepointVerifier, since we don't safepoint.
The other usecase seems to be, I'm in java don't do VM things, and someone tried
to protected that with a NoHandleMark. There many ways to create a much more
robust check for that.
But a discussion for another time.
>
> Thanks,
> David
>
>> Issue:
>> https://bugs.openjdk.java.net/browse/JDK-8228758
>>
>> Changeset:
>> http://cr.openjdk.java.net/~rehn/8228758/webrev/
>>
>> Passes t1-3 (and performance benchmarks)
>>
>> Thanks, Robbin
More information about the hotspot-runtime-dev
mailing list