RFR(S): 8163994: Nightly test crashed in jvmtiAllocate

[David Holmes]

Hi Serguei,

On 30/08/2016 4:46 AM, serguei.spitsyn at oracle.com wrote:
> Chris and David,
>
> We had a private discussion about this bug with Dmitry last week.
> I initially suggested to close it as a dup of JDK-8134103 but then
> agreed with a fix replacing crash symptom with AGENT_ERROR_INTERNAL.
> I still have some doubt if it makes sense, as it does not look as
> important.
>
> Now, it seems you also prefer to close this bug as a dup.
> But let's check your opinion on the Dmitry's reasoning below.

The problem is that the "fix" still doesn't guarantee that we will get 
the more informative AGENT_ERROR_INTERNAL. The whole situation is racy.

But I won't block it and don't want to waste time arguing over it. So if 
Dmitry wants to proceed then you can count this as Reviewed.

Thanks,
David
-----

> Thanks,
> Serguei
>
>
> On 8/29/16 06:12, Dmitry Samersoff wrote:
>> Chris & David,
>>
>> JVMTI_ERROR_WRONG_PHASE problem is complicated and requires significant
>> work probably on both JDWP and JVMTI side. Serguei plan to do it as a
>> part of JDK-8134103 and not for JDK 9.
>>
>> So yes, we can close this one as a dup of JDK-8134103 - it has the same
>> root cause and should be addressed as the part of JDK-8134103
>> (particularly, we have to cleanup ignore_vm_death logic)
>>
>> But the crash is observed only once in a nightly, so my intention is to
>> save us a bit of time next time when this situation happens.
>>
>> i.e. before the changes we get JVMTI_ERROR_WRONG_PHASE message and
>> *crash*, after the changes  we get JVMTI_ERROR_WRONG_PHASE message
>> and AGENT_ERROR_INTERNAL message.
>>
>>
>> -Dmitry
>>
>>
>>
>> On 2016-08-29 09:43, Chris Plummer wrote:
>>> On 8/28/16 6:14 PM, David Holmes wrote:
>>>> On 27/08/2016 7:35 AM, Chris Plummer wrote:
>>>>> Hi Dmitry,
>>>>>
>>>>> Although the fix is addressing the specific issue described in the
>>>>> bug,
>>>>> what about the general issue of referencing gdata after a call to
>>>>> cbEarlyVMDeath(). Do more references to gdata need to be protected?
>>>>>
>>>>> Also, is there the possibility of a multi-threading race condition
>>>>> here?
>>>>> Could gdata be cleared by another thread after it is checked?
>>>> Certainly. This really isn't fixing anything just adding a bailout
>>>> check before the crashing code. We can still crash and not be any the
>>>> wiser as to why.
>>>>
>>>> Not sure I really see the point of doing this instead of closing this
>>>> as a dup of JDK-8134103 and fixing things properly.
>>> It it correct to say that Dmitry is fixing a bug exposed by JDK-8134103,
>>> or that he is temporarily working around something that is not a true
>>> bug, but is indirectly caused by JDK-8134103. I'm not sure, but the
>>> answer will dictate the correct course of action here.
>>>
>>> Chris
>>>> David
>>>>
>>>>> thanks,
>>>>>
>>>>> Chris
>>>>>
>>>>> On 8/26/16 4:00 AM, Dmitry Samersoff wrote:
>>>>>> Everybody,
>>>>>>
>>>>>> Please review the fix.
>>>>>>
>>>>>> http://cr.openjdk.java.net/~dsamersoff/JDK-8163994/webrev.02/
>>>>>>
>>>>>> *Problem*
>>>>>>
>>>>>> Under some circumstances, when JVMTI_ERROR_WRONG_PHASE(112) is
>>>>>> received,
>>>>>> jvmtiAllocate could be called after call to cbEarlyVMDeath.
>>>>>>
>>>>>> cbEarlyVMDeath set gdata->jvmti to NULL, so jvmtiAllocate crashes.
>>>>>>
>>>>>> The problem appears only once in nightly testing and I was not
>>>>>> able to
>>>>>> reproduce it locally.
>>>>>>
>>>>>> *Solution*
>>>>>>
>>>>>> Guard added to jvmtiAllocate to get meaningful error message
>>>>>> instead of
>>>>>> crash.
>>>>>>
>>>>>> These fix doesn't fix root cause - JVMTI_ERROR_WRONG_PHASE problem is
>>>>>> going to be addressed under JDK-8134103.
>>>>>>
>>>>>> -Dmitry
>>>>>>
>>
>