On 25/03/2019 3:06 pm, Ioi Lam wrote:
On 3/24/19 9:14 PM, David Holmes wrote:
Hi Ioi,
On 25/03/2019 1:46 pm, Ioi Lam wrote:
https://bugs.openjdk.java.net/browse/JDK-8217347
http://cr.openjdk.java.net/~iklam/jdk13/8217347-appcds-InstrumentationTest-timeout.v01/
This test case used VirtualMachine.list() to find the pid of the the
child
process. However, as David Holmes commented in the bug report, it's
suspected that this call may lead to timeout.
In any case, since Process.pid() has been added since JDK 9, it's
better to call this
API directly. That way, we can avoid unnecessary dependency on
VirtualMachine.list().
In so far as I can see how you replaced the VM.list() with
Process.pid() this seems okay. I'm having trouble actually
understanding the handshake being used to control things though. I
would have expected the target VM to create the flagfile to indicate
it is ready for attaching, then it would wait for the flagfile to be
deleted.
Hi David,
As far as I can tell, the handshake is this: the main test process
creates the flag file and passes the name of the flag file to the child
process (whose main class is InstrumentationApp). The child process will
wait until the main process deletes the flag file.
After the child process is launched, the main process attaches to it
(using the child's pid), and then deletes the flag file. At that point,
the child will resume execution.
Okay but the problem is that the main process can try to attach to the
child process before the child is ready to be attached to. This is a day
one limitation of the attach-on-demand process. If the child process
created the flags file (using the name supplied by the main process) the
main process could wait until it sees it to do the attach.
Anyway this is not directly related to your fix, just be aware this test
can fail in obscure ways as the child process may silently disappear if
the SIGQUIT used to attach-on-demand arrives too soon.
BTW, I found this comment to be out-dated so I changed it:
- // At this point, the child process is not yet launched. Note that
- // TestCommon.exec() and OutputAnalyzer.OutputAnalyzer() both
block
- // until the child process has finished.
- //
- // So, we will launch a AgentAttachThread which will poll the
system
- // until the child process is launched, and then do the
attachment.
- // The child process is uniquely identified by having flagFile
in its
- // command-line -- see AgentAttachThread.getPid().
+ // At this point, the child process is not yet launched.
+ // We will launch a AgentAttachThread which will wait until
+ // the child process's pid is known, and then do the attachment.
Okay but the comment is out of date because you now return the process
directly, which means you no longer need the AgentAttachThread at all,
you can do the attach logic directly from the main thread.
Cheers,
David
-----
AgentAttachThread t = new AgentAttachThread(flagFile, agentJar);
t.start();
return t;
Thanks
- Ioi
Thanks,
David
Tested with tiers{1,2,3}. Also removed some unnecessary @module and
import lines.
Thanks
- Ioi
