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

[serguei.spitsyn at oracle.com]

I don't know the exact answer but I thought one motivation was
to avoid test/agent interference and so make it more stable.

Thanks,
Serguei


On 10/4/16 14:59, Chris Plummer wrote:
> Do you know why "othervm" is needed for the other j.l.i tests?
>
> Chris
>
> On 10/4/16 2:50 PM, serguei.spitsyn at oracle.com wrote:
>> 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
>>>>
>>>
>>
>