Hi David,

Thank you for the comment.

Dmitry,

It seems, Chris and David are not comfortable with current fix.
Are you Ok to close this bug as a dup?

Thanks,
Serguei


On 8/29/16 20:05, David Holmes wrote:
Hi Serguei,

On 30/08/2016 4:46 AM, serguei.spit...@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




Reply via email to