Hi Serguei,
This looks good to me. One not below.
On 2/09/2017 6:30 PM, [email protected] 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
