RFR 8181299/10, Several jdk tests fail with java.lang.NoClassDefFoundError: jdk/test/lib/process/StreamPumper

[Ioi Lam]

I agree with what Daniel said. Even without explicit @build tags (as in 
the reproducer in CODETOOLS-790198), if you use something like

     @run main RedefineClassHelper

that would cause an implicit invocation of "@build test/lib", because 
RedefineClassHelper.java is part of test/lib

So it's not possible to avoid @build altogether (even if you're not 
using reflection).

=====

More explanation of the jtreg bug:

In the CODETOOLS-790198 reproducer's case, another test 
(ModifyAnonymous.java) uses jdk.test.lib.compiler.InMemoryJavaCompiler 
without an explicit @build.

Later, when RedefineRunningMethodsWithResolutionErrors.java is executed 
and runs "@run main RedefineClassHelper", the jtreg bug causes 
classes/test/lib to be partially compiled -- RedefineClassHelper.class 
is there, but InMemoryJavaCompiler.class is missing.

Sure, according to the jtreg docs, "@build 
jdk.test.lib.compiler.InMemoryJavaCompiler" should be added to 
ModifyAnonymous.java. However, when you test fails because ANOTHER TEST 
forgets to add an @build, and you are looking at a sea of over 1000 test 
cases, you're completely lost.

So what we have is a jtreg rule that says "you should ...", but there's 
no enforcement (every test runs perfectly fine by itself), and when 
failure happens there's no diagnostic that tells you who's to blame. 
Seems like a perfect recipe for anarchy.

- Ioi


On 6/2/17 2:19 AM, Daniel Fuchs wrote:
> Hi guys,
>
> The jtreg bug really needs to be fixed.
> What I hear is that adding an explicit @build in one test
> can make an unrelated test that depends on the same library
> but doesn't have the explicit @build fail (and possibly
> randomly and intermittently depending of the order in
> which tests are run).
>
> This is very unintuitive, and the 'obvious' (thouhj maybe
> wrong) fix for anyone stumbling on the issue would be to fix
> the failing test by adding the explicit @build - not grep
> the whole test base in search for a test that might have an
> explicit @build, which as pointed elsewhere might well be
> legitimate if the test is using reflection.
>
> So until the jtreg bug is fixed, I am not at all sure that
> removing all the explicit @build is the correct thing to do,
> as it's still bound to make existing unrelated tests fail
> randomly if new tests with an explicit @build are added
> later on...
>
> my2c
>
> -- daniel
>
> On 01/06/2017 23:37, Ioi Lam wrote:
>>
>>
>> On 6/1/17 1:17 PM, Igor Ignatyev wrote:
>>>> On Jun 1, 2017, at 1:20 AM, Chris Hegarty 
>>>> <chris.hegarty at oracle.com> wrote:
>>>>
>>>> Igor,
>>>>
>>>>> On 1 Jun 2017, at 04:32, Igor Ignatyev <igor.ignatyev at oracle.com> 
>>>>> wrote:
>>>>>
>>>>> Hi Felix,
>>>>>
>>>>> I have suggested the exact opposite change[1-3] to fix the same 
>>>>> problem.
>>>> I’m sorry, but this is all just too confusing. After your change, 
>>>> who, or what, is
>>>> responsible for building/compiling the test library dependencies?
>>> jtreg is responsible, there is an implicit build for each @run, and 
>>> jtreg will analyze a test class to get transitive closure for static 
>>> dependencies, hence you have to have @build only for classes which 
>>> are not in constant pool, e.g. used only by reflection or whose 
>>> classnames are only used to spawn a new java instance.
>>
>>
>> I suspect the problem is caused by a long standing bug in jtreg that 
>> results in library classes being partially compiled. Please see my 
>> evaluation in
>>
>> https://bugs.openjdk.java.net/browse/CODETOOLS-7901986
>>
>> In the bug report, there is test case that can reliably reproduce the 
>> NoClassDefFoundError problem.
>>
>> I think adding all the @build commands in the tests are just 
>> band-aids. Things will break unless every test explicitly uses @build 
>> to build every class in every library that they use, including all 
>> the private classes that are not directly accessible by the test cases.
>>
>> For example: doing this may be enough for now:
>>
>>       * @build jdk.test.lib.process.*
>>
>> But what if in the future, jdk.test.lib.process is restructured to 
>> have a private package jdk.test.lib.process.hidden? To work around 
>> CODETOOLS-7901986, all the test cases that must be modified to the 
>> following, which unnecessarily exposes library implementation details 
>> to the library users:
>>
>>       * @build jdk.test.lib.process.* jdk.test.lib.process.hidden.*
>>
>> Just imagine this -- "in order to use malloc() you must explicitly 
>> build not only malloc(), but also sbrk() ... and every other function 
>> in libc". That seems unreasonable to me.
>>
>> By the way, we made a fix in the HotSpot tests (see 
>> https://bugs.openjdk.java.net/browse/JDK-8157957) that got rid of 
>> many (but not all) of the NoClassDefFoundErrors by *removing* the 
>> @build lines .....
>>
>> My proposal is, instead of just adding @build for band-aid, we should 
>> fix CODETOOLS-7901986 instead.
>>
>> Thanks
>> - Ioi
>>
>>
>>>>
>>>> Test library code has no @modules tags, so does not explicitly 
>>>> declare its
>>>> module dependencies. Instead module dependencies, required by test
>>>> library code, are declared in the test using the library. If we 
>>>> wildcard, or
>>>> otherwise leave broad build dependencies, from tests then there is no
>>>> way to know what new module dependencies may be added in the future.
>>>> That is, one of, the reason(s) I asked Felix to be explicit about 
>>>> the build
>>>> dependencies.
>>> having explicit builds does not really help w/ module dependency, if 
>>> someone change a testlibrary class so it starts to depend on another 
>>> testlibrary class, jtreg will implicitly build it and if this class 
>>> has some module dependencies, you will have to reflect them in the 
>>> test.
>>>
>>> generally speaking, I don't like having explicit build actions 
>>> because build actions themselves are implicit, so they don't really 
>>> help, it's still will be hard to spot missed explicit builds. not 
>>> having (unneeded) explicit builds is an easy rule to follow and we 
>>> can easily find all places which don't follow this rule by grep.
>>>
>>> -- Igor
>>>> -Chris.
>>>>
>>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8181391
>>>>> [2] 
>>>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-June/048012.html 
>>>>>
>>>>> [3] 
>>>>> http://cr.openjdk.java.net/~iignatyev//8181391/webrev.00/index.html
>>
>