RFR(XS): 8167001: [TESTBUG] java/lang/instrument/DaemonThread/TestDaemonThread.java fails when run by jprt

[serguei.spitsyn at oracle.com]

Hi Chris,

The createJavaProcessBuilder() approach is Ok with me.
Just curios why the "othervm" is not used for this case.
My theory is that it is because this test is located in sub-folder,
so that it was forgotten in some update. :)

Thanks,
Serguei


On 10/4/16 14:33, Chris Plummer wrote:
> Hi Serguei,
>
> Yes, adding othervm also fixes the problem. What also works is adding 
> the following VM arguments when calling createJavaProcessBuilder().:
>
>     "-cp", System.getProperty("test.class.path")
>
> or
>
>     "-cp", System.getProperty("java.class.path")
>
> I've seen the above used, othervm used, and relying on 
> ProcessTools.createJavaProcessBuilder() to add the classpath, either 
> implicitly as is done by default in hotspot/test, or explicitly by 
> passing true as the first arg as is done in jdk/test.
>
> My preferences is the createJavaProcessBuilder() approach. I worry 
> that othervm is relying on a side affect of its use, and is not on a 
> direct solution to the problem. I still haven't gotten an explanation 
> as to why othervm causes CLASSPATH to be exported, and agentvm causes 
> -classpath to be used instead. Using the createJavaProcessBuilder() 
> approach will continue to work even if othervm stopped exporting 
> CLASSPATH.
>
> thanks,
>
> Chris
>
> On 10/4/16 2:18 PM, serguei.spitsyn at oracle.com wrote:
>> Hi Chris,
>>
>> It looks good to me.
>> But I have a question.
>> Would it also work if we add "othervm" to the test 
>> TestDaemonThread.java?
>> It seems other j.l.i tests are run in this mode.
>>
>> Thanks,
>> Serguei
>>
>>
>> On 10/4/16 10:43, Chris Plummer wrote:
>>> Hello,
>>>
>>> Please review the following:
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8167001
>>> http://cr.openjdk.java.net/~cjplummer/8167001/webrev.00/webrev.jdk/
>>>
>>> This fixes an issue with the classpath not being passed to the 
>>> subprocess, resulting in the main class for the subprocess not being 
>>> found. It only turns up when running the test with "agentvm", which 
>>> is the default when running the test via test/Makefile (which is 
>>> what jprt uses). "othervm" seems to be the default when running 
>>> jtreg directly from the command line, so it doesn't see this problem.
>>>
>>> Although there are still ongoing discussion of jtreg classpath 
>>> definition differences between "agentvm" mode and "othervm" mode, 
>>> existing tests fix this issue by explicitly passing the classpath to 
>>> the subprocess. This is usually done by passing "true" as the first 
>>> argument to ProcessTools.createJavaProcessBuilder(), so I'd like to 
>>> go with this fix for now. This is actually the default behavior for 
>>> hotspot/test. For jdk/test the default is "false" so you need to 
>>> explicitly pass "true".
>>>
>>> Tested by running with jprt under conditions where the test was 
>>> previously failing. Also tested by running the test directory 
>>> locally using jdk/Makefile (which previously was failing).
>>>
>>> thanks,
>>>
>>> Chris
>>
>