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

[Chris Plummer]

I think in general that's the reason for using othervm. For this 
particular test it's probably not needed since all the real work is done 
by the creating subprocesses.

Chris

On 10/4/16 3:18 PM, serguei.spitsyn at oracle.com wrote:
> 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
>>>>>
>>>>
>>>
>>
>