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

[Jonathan Gibbons]

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
>>
>