RFR(S) 8177901: JDWP exit error JVMTI_ERROR_WRONG_PHASE(112): on checking for an interface

[David Holmes]

Hi Serguei,

This looks good to me. One not below.

On 2/09/2017 6:30 PM, serguei.spitsyn at oracle.com wrote:
> Please, review a fix for:
> https://bugs.openjdk.java.net/browse/JDK-8177901
> 
> Webrev:
> http://cr.openjdk.java.net/~sspitsyn/webrevs/2017/hotspot/8177901-jdi-wrong_phase.1

src/jdk.jdwp.agent/share/native/libjdwp/debugInit.c

+     // Release commandLoop vmDeathLock if necessary
+     void commandLoop_exitVmDeathLockOnError(void);
+     commandLoop_exitVmDeathLockOnError();

The declaration should be in the eventHelper.h header file. It looks 
really odd to declare the function then call it.

Thanks,
David
-----

> 
> Summary:
> 
>    The approach to fix this is to introduce a new lock vmDeathLock in 
> eventHelper.c
>    and use it to sync the commandLoop with the JVMTI event callback 
> cbVMDeath
>    the same way as it was done for debugLoop_run.
>    (see the fix of: https://bugs.openjdk.java.net/browse/JDK-8134103)
> 
>    One potential issue with it is that the commandLoop() transitively 
> uses some helper
>    functions (e.g. from util.c) that use the macro EXIT_ERROR, and so, 
> can abort.
>    It seems, in such a case the vmDeathLock will remain locked, and so,
>    the cbVMDeath() will block on it causing a deadlock.
>    Note, that these helper functions can be also called from different 
> contexts/threads
>    (not from the commandLoop thread only). In such contexts the 
> commandLoop vmDeathLock
>    is not being entered, and so, should not be exited.
> 
>    To fix this potential issue, new function, 
> commandLoop_exitVmDeathLockOnError(),
>    is introduced, and it is called from the debugInit_exit().
> The commandLoop_exitVmDeathLockOnError() always checks if current thread is
>    the commandLoop thread and only in such a case unlocks the vmDeathLock.
> 
> Testing:
>    Ran the nsk.jdi, nsk.jdwp and jtreg jdk_jdi for both release and 
> fastdebug builds.
>    All tests are passed.
> 
> Thanks,
> Serguei
>