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

[Chris Plummer]

Hi Jon,

Thanks for the explanation.

Chris

On 10/4/16 3:30 PM, Jonathan Gibbons wrote:
> Chris, Serguei,
>
> CLASSPATH is used in othervm mode for historical reasons, and includes 
> the test-specific classes.
>
> Environment variables cannot be modified during execution, and so 
> cannot contain test-specific classes in agent mode, since the agent VM 
> is reused for different tests. Therefore although the env variable is 
> set, it doesn't (cannot) include test classes.
>
> As a general technique for both the launcher and for javac, I 
> recommend avoiding environment variables like CLASSPATH, and using 
> options like -cp/-classpath.
>
> -- Jon
>
> On 10/04/2016 02:33 PM, 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
>>>
>>
>