RFR: 8321164: javac with annotation processor throws AssertionError: Filling jrt:/... during JarFileObject[/...]

[Jan Lahoda]

On Wed, 6 Dec 2023 22:03:29 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> This is a latent bug revealed by the recent JDK-8225377/https://git.openjdk.org/jdk/commit/de6667cf11aa59d1bab78ae5fb235ea0b901d5c4.
>> 
>> The problem (to my understanding) is as follows:
>>  - `ClassReader` cannot be used recursively, i.e. while completing one class from a classfile, we cannot complete another one. `ClassReader` carefully avoids situations like this.
>>  - when reading an anonymous class, the `EnclosingMethod` attribute is read, and the enclosing method must be found. `ClassReader` uses a special `ClassReader. isSameBinaryType` method, that is supposed to avoid completion:
>> https://github.com/openjdk/jdk/blob/3edc24a71d29632e0a2166a64fc25ce83f631b47/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java#L1401
>>  - the `isSameBinaryType` method uses erasure, and if the given type has type annotations, the erasure will try to remove the annotations:
>> https://github.com/openjdk/jdk/blob/3edc24a71d29632e0a2166a64fc25ce83f631b47/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java#L2423
>> 
>> The problem is that this method uses `Type.getKind()` which completes.
>> 
>> The proposal in this patch is to avoid the completion by not using `getKind`, but rather `getTag`. While I am not completely sure why the code avoids doing `dropMetadata` for some types, I tried to replicate the existing behavior. One caveat is that certain types may have `getTag() == CLASS`, but `getKind() == ERROR` (e.g. if the class is not found). The new code is not replicating this aspect, as we cannot reliably determine whether a `ClassType` is erroneous or not without completing it. But, it is not clear to me why dropping annotations from an erroneous `ClassType` should be problematic.
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java line 2424:
> 
>> 2422:                              ARRAY, MODULE, TYPEVAR, WILDCARD:
>> 2423:                             return s.dropMetadata(Annotations.class);
>> 2424:                         case VOID, METHOD, PACKAGE, FORALL, DEFERRED, BOT,
> 
> Not that it should matter much (as it cannot be annotated) but the `BOT` type seemed to receive a different treatment before (the kind for that is `NULL`). Separately, I'm a bit hazy of the rationale behind the logic here. E.g. is it an optimization to avoid removing metadata that we know isn't there, or is it something else? Anyway, not something to fix now (mostly thinking aloud)

Uh, I somehow missed the `NULL`. Thanks for noticing. My intent here is not to change the behavior if possible, so I restored the `NULL`/`BOT` behavior as it was before.

As for the logic, I agree, I don't know why it is written like this. We could possibly try to cleanup early in a release and test.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/17001#discussion_r1418465050