|
Hi Yusamasa,
Thank you for the clarifications!
It looks good to me.
Thanks,
Serguei
On 5/24/19 03:00, Yasumasa Suenaga wrote:
Hi Serguei,
Other changes are included to improve
reliability of testcase.
DebugServer would show "Debugger attached ..." when attach
operation is completed. So I change its value.
Then the testcase should wait until debugd
process is terminated normaly. So I added waitFor() call.
Null check of "app" would be done in stopApp().
So I remove it.
Thanks,
Yasumasa
Hi
Yasumasa,
I'm a little bit confused with this fix.
If this test is failed on Windows only then the line:
+ * @requires
os.family != "windows"
prevents the test to be run on Windows (which looks Okay
to me).
Then why are there other fixes (excluding the comment
with bug numbers)?
Your summary below only tells about problem on Windows.
What was motivation
for these changes?
Thanks,
Serguei
On 5/23/19 17:23, Yasumasa Suenaga wrote:
Hi all,
Please review this change.
This webrev has passed all tests on submit repo
(mach5-one-ysuenaga-JDK-8224252-3-20190523-0532-2647154).
JBS: https://bugs.openjdk.java.net/browse/JDK-8224252
webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8224252/webrev.00/
JDK-8163805 was suppose to fix a timeout issue with this test and also
removed it off the problemlist. However, it still times out on
windows-x64 about 1/3 of the time. It passes every time on all the
other platforms.
Root cause of this is `jhsdb debugd` could not be exited normally.
`jhsdb debugd` (internally, DebugServer.java) would detach from
debuggee at shutdown hook. So jhsdb process need to be exited
normally.
`jhsdb debugd` could not exit itself without external operation (e.g.
CTRL+C, signals). Thus we use Process::destroy for it.
In the case of Windows, Process::destroy calls TerminateProcess()
Windows API to terminate process. However it would terminate target
process immediately and it would not give a chance to kick shutdown
hook. (It is documented on Javadoc of Runtime::addShutdownHook)
Thanks,
Yasumasa
|
