Looks good! Thanks, /Staffan
On 1 jul 2014, at 09:37, Erik Gahlin <erik.gah...@oracle.com> wrote: > Updated webrev: > http://cr.openjdk.java.net/~egahlin/8028474_5/ > > See comments inline. > > Staffan Larsen skrev 2014-06-26 13:22: >> Indentation around JavaProcess.getId() is weird. >> > Fixed >> JavaProcess.getPid/setPid/pid do not appear to be used. >> > Fixed > >> JavaProcess.waitForRemoval: How about using timestamps (currentTimeMillis()) >> before the loop and for each ite >> ration to determine if the timeout has expired (instead of "time+=100”)? >> > The code now uses currentTimeMillis(). Premature timeouts due to clock > changes can be deduced from the log. > > I also cleaned up the log for releaseStarted/releaseTerminated > >> nit: missing empty line before line 139, method releaseStarted(). >> > Fixed > > Thanks > Erik > >> /Staffan >> >> >> On 26 jun 2014, at 00:56, Erik Gahlin <erik.gah...@oracle.com> wrote: >> >>> It didn't work. >>> >>> There's no termination notification, if I use Process#destroyForcibly(). I >>> believe the hsperfdata file is left behind so it will not be able to detect >>> that the process has died. >>> >>> Here is an updated version that renames some variable/methods. >>> >>> http://cr.openjdk.java.net/~egahlin/8028474_3/ >>> >>> Thanks >>> Erik >>> >>> Erik Gahlin skrev 2014-06-18 09:37: >>>> Didn't know about destroyForcibly(). I could try that. >>>> >>>> Erik >>>> >>>> Staffan Larsen skrev 18/06/14 09:26: >>>>> Erik, >>>>> >>>>> How about using Process.destroyForcibly() as a means to terminate the >>>>> process instead of using files for signaling? >>>>> >>>>> /Staffan >>>>> >>>>> On 17 jun 2014, at 23:13, Erik Gahlin <erik.gah...@oracle.com> wrote: >>>>> >>>>>> Yes, very weird >>>>>> >>>>>> Could have been that I needed the tools.jar in the child processes, or >>>>>> it could have been something else. If I remember correctly, I got a CNF >>>>>> that I didn't get with the shell script, but it was few months back. >>>>>> >>>>>> Anyway, I retried with JPRT and now it works without the shell script. >>>>>> >>>>>> http://cr.openjdk.java.net/~egahlin/8028474_2/ >>>>>> >>>>>> Erik >>>>>> >>>>>> Staffan Larsen skrev 2014-06-16 13:49: >>>>>>> >>>>>>> On 16 jun 2014, at 12:32, Erik Gahlin <erik.gah...@oracle.com> wrote: >>>>>>> >>>>>>>> Yekaterina Kantserova skrev 13/06/14 12:59: >>>>>>>>> Erik, >>>>>>>>> >>>>>>>>> is there some reason why we need to keep MonitorVmStartTerminate.sh? >>>>>>>>> I've moved the JTreg header to MonitorVmStartTerminate.java >>>>>>>> Hi Katja, >>>>>>>> >>>>>>>> That's how I did the test initially, and it works locally, but I could >>>>>>>> never get it to work in JPRT without the shell script. I believe the >>>>>>>> tools.jar is not on the class path. >>>>>>> >>>>>>> That is weird. I see other tests that depend in tools.jar and they work >>>>>>> fine. >>>>>>> >>>>>>> /Staffan >>>>>>> >>>>>>> >>>>>>>> >>>>>>>> Erik >>>>>>>>> >>>>>>>>> /* >>>>>>>>> * @test >>>>>>>>> * @bug 4990825 >>>>>>>>> * @summary attach to external but local JVM processes >>>>>>>>> * @library /lib/testlibrary >>>>>>>>> * @build jdk.testlibrary.* >>>>>>>>> * @run main MonitorVmStartTerminate >>>>>>>>> */ >>>>>>>>> >>>>>>>>> and the test works just fine. >>>>>>>>> >>>>>>>>> The JTreg run contains all pathes and system properties >>>>>>>>> MonitorVmStartTerminate.sh tries to construct: >>>>>>>>> ${JAVA} ${TESTVMOPTS} -Dtest.jdk=${TESTJAVA} >>>>>>>>> -Dtest.classes=${TESTCLASSES} -classpath ${CP} MonitorVmStartTerminate >>>>>>>>> >>>>>>>>> See the log attached. >>>>>>>>> >>>>>>>>> Note @build jdk.testlibrary.* instead of @build >>>>>>>>> jdk.testlibrary.ProcessTools to make sure all testlibrary classes are >>>>>>>>> compiled >>>>>>>>> to the right place when running tests concurrently. >>>>>>>>> >>>>>>>>> Thanks, >>>>>>>>> Katja (Not a Reviewer) >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> On 06/12/2014 12:37 AM, Erik Gahlin wrote: >>>>>>>>>> Hi, >>>>>>>>>> >>>>>>>>>> Could I have a review of a test that has been failing >>>>>>>>>> intermittently. The test now uses files for synchronization >>>>>>>>>> instead of waiting for a process that sleeps. >>>>>>>>>> >>>>>>>>>> Webrev: >>>>>>>>>> http://cr.openjdk.java.net/~egahlin/8028474/ >>>>>>>>>> >>>>>>>>>> Bug: >>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8028474 >>>>>>>>>> >>>>>>>>>> Description: >>>>>>>>>> >>>>>>>>>> The test starts ten Java processes, each with a unique id. >>>>>>>>>> >>>>>>>>>> Each process creates a file named after the id and then it waits >>>>>>>>>> for >>>>>>>>>> the test to remove the file, at which the Java process exits. >>>>>>>>>> >>>>>>>>>> The processes are monitored by the test to make sure notifications >>>>>>>>>> are sent when processes are started/terminated. >>>>>>>>>> >>>>>>>>>> To avoid Java processes being left behind, in case of an unexpected >>>>>>>>>> failure, shutdown hooks are registered that remove files when the >>>>>>>>>> test >>>>>>>>>> exits. If files are not removed, i.e. due to a JVM crash, >>>>>>>>>> the Java processes will exit themselves after 1000 s. >>>>>>>>>> >>>>>>>>>> Thanks >>>>>>>>>> Erik >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> >
