[15] RFR: 8246381: VM crashes with "Current BasicObjectLock* below than low_mark"

[Jamsheed C M]

Hi David,

On 16/07/2020 05:20, David Holmes wrote:
>>>
>>> Hi,
>>>
>>> Async handling at method entry requires it to be aware of 
>>> synchronization(like whether it is doing async handling before lock 
>>> acquire or after)
>>>
>>> This is required as exception handler rely on this info for 
>>> unlocking.  Async handling code never had this special condition 
>>> handled and it worked most of the time as we were using biased 
>>> locking which got disabled by [1]
>>>
>>> There was one other issue reported in similar time[2]. This issue 
>>> got triggered in test case by [3], back to back extra safepoint 
>>> after suspend and TLH for ThreadDeath. So in this setup both 
>>> PopFrame request and Thread.Stop request happened together for the 
>>> test scenario and it reached java method entry with 
>>> pending_exception set.
>>>
>>> I have done a partial fix for the issue, mainly to handle production 
>>> mode crash failures(do not unlock flag related ones)
>>>
>>> Fix detail:
>>>
>>> 1) I save restore the "do not unlock" flag in async handling.
>
> Sorry but you completely changed the fix compared to what we discussed 
> and what I pre-reviewed! What happened to changing from JRT_ENTRY to 
> JRT_ENTRY_NOASYNC? It is going to take me a lot of time and effort to 
> determine that this save/restore of the "do not unlock flag" is 
> actually correct and valid!

I tried JRT_ENTRY to JRT_ENTRY_NOASYNC. but unfortunately that made some 
tests to fail(logs in JBS), I didn't investigate it in detail, but what 
I presume is

pending_async_exception is set for those failing scenarios but as we 
have  disabled async handling in some prominent code paths, the 
exception is never delivered.

>>>
>>> 2) Return for floating pending exception for some cases(PopFrame, 
>>> Early return related). This is debug(JVMTI) feature and floating 
>>> exception can get cleaned just like that in present compiler request 
>>> and deopt code.
>
> What part of the change addresses this?

It doesn't address this issue completely. As it requires other changes 
in compilation request path(c1,interpreter) and deopt.

Just made changes to interpreter part(compilation request part). that 
fixes interpreter part partially.

  JRT_ENTRY(nmethod*,
            InterpreterRuntime::frequency_counter_overflow_inner(JavaThread* thread, address branch_bcp))
+ if (HAS_PENDING_EXCEPTION) {
+ return NULL;
+ }   JRT_ENTRY(void, InterpreterRuntime::profile_method(JavaThread* thread))
+ if (HAS_PENDING_EXCEPTION) {
+ return;
+ }

Best regards

Jamsheed

>
> Thanks,
> David
> -----