QuarkusClassLoader ignores method hierarchy when overriding overloaded methods in v1.13.x

[pegrc]

We seem to have encountered a bug, but it could very well be a configuration issue on our side as well. However, it's symptomatic that it only started happening after upgrading Quarkus to 1.13.x. Any help would be appreciated.

Describe the bug

We are extending an abstract class and thus overriding an abstract method from a third party library(https://github.com/jboss-logging/jboss-logmanager/blob/master/core/src/main/java/org/jboss/logmanager/ExtFormatter.java). This abstract method takes a child class(ExtLogRecord), and there's also an overloaded final method with the parent class(LogRecord). Since upgrading to Quarkus 1.13.x we get a java.lang.VerifyError on build that we cannot override a final method thrown by the QuarkusClassLoader. The verification incorrectly assumes we're trying to override the final(non-abstract) parent class method, instead of the abstract child one. The problem only seems to be present in dev mode(thus we believe is related to the changes done to QuarkusClassLoader);

Expected behavior

The verification passes, as the override refers to the correct method. This should be possible and is expected use of the library, and works without any problems with Quarkus 1.12.x.

Actual behavior

The verification fails, as the application assumes we're trying to override the wrong method.

To Reproduce

Try extending the class mentioned in the description(or a custom one with the same setup), and run it in dev mode.

Steps to reproduce the behavior:

1. Implement extended class with same setup/behavior.
2. Run Quarkus in dev mode.

Example project

example.zip

Simple building the project and running quarkusDev should reproduce the issue.

Screenshots

The overloaded methods of the class we're trying to override:

Our method:

The error(our package and class names have been omitted):

Environment (please complete the following information):

Output of uname -a or ver

Darwin [user] 19.6.0 Darwin Kernel Version 19.6.0: Tue Jan 12 22:13:05 PST 2021; root:xnu-6153.141.16~1/RELEASE_X86_64 x86_64

Also updated to latest MacOS(Big Sur) but problem still persists.

Output of java -version

openjdk version "11.0.10" 2021-01-19
OpenJDK Runtime Environment AdoptOpenJDK (build 11.0.10+9)

Quarkus version or git rev

1.13.2.Final

Build tool (ie. output of mvnw --version or gradlew --version)

Gradle 6.8.3

Additional context

Could be a macOS specific issue? One of our colleagues on Windows reported to not have an issue.

Edit: our colleague confirmed it also doesn't work on Windows. Provided sample project.

[pegrc]

Label is wrong it should be macOS.

[famod]

/cc @stuartwdouglas

[geoand]

Any chance we can get a simple reproducer?

[pegrc]

Hey @geoand I've provided a small project as a zip in the original post which includes the problematic library.

[geoand]

Thanks

[stuartwdouglas]

@mkouba sounds like an arc issue, looks like ArC is trying to override a final method.

[geoand]

@stuartwdouglas I am just now looking into it, but that's also what I gathered from looking at the error message. I want to dump the actual output to see

[geoand]

So indeed, org.simple.reproducer.services.ImplementedService_Subclass is overriding public final String format(LogRecord var1):

    public String format(LogRecord var1) {
        Object[] var5 = new Object[]{var1};
        if (this.arc$constructed) {
            ImplementedService_Subclass$$function$$2 var4 = new ImplementedService_Subclass$$function$$2(this);

            try {
                InterceptedMethodMetadata var2 = this.arc$2;
                Method var3 = var2.method;
                List var6 = var2.chain;
                Set var7 = var2.bindings;
                return (String)InvocationContexts.performAroundInvoke(this, var3, (Function)var4, var5, var6, var7);
            } catch (RuntimeException var10) {
                throw (Throwable)var10;
            } catch (Exception var11) {
                throw (Throwable)(new ArcUndeclaredThrowableException("Error invoking subclass method", (Throwable)var11));
            }
        } else {
            return this.format$$superforward1(var1);
        }
    }

@mkouba ^

[mkouba]

Hm, IIRC we try to remove the final modifier from the intercepted methods. @geoand It was originally implemented by you ;-). Could it be a problem because jboss logmanager classes are loaded by CL that does not support transformation?

This should be possible and is expected use of the library, and works without any problems with Quarkus 1.12.x.

Since 1.13 the container monitors business method invocations in the development mode by default. You can disable monitoring via quarkus.arc.dev-mode.monitoring-enabled=false and the error should go away because your bean is normally not intercepted.

[geoand]

Hm, IIRC we try to remove the final modifier from the intercepted methods. @geoand It was originally implemented by you ;-). Could it be a problem because jboss logmanager classes are loaded by CL that does not support transformation?

Of course I did... :(.
So yeah, because the logmanager is loaded parent first, the transformations are not being applied.

This should be possible and is expected use of the library, and works without any problems with Quarkus 1.12.x.

Since 1.13 the container monitors business method invocations in the development mode by default. You can disable monitoring via quarkus.arc.dev-mode.monitoring-enabled=false and the error should go away because your bean is normally not intercepted.

Hm, maybe we should document this somewhere?

[mkouba]

Hm, IIRC we try to remove the final modifier from the intercepted methods. @geoand It was originally implemented by you ;-). Could it be a problem because jboss logmanager classes are loaded by CL that does not support transformation?

Of course I did... :(.
So yeah, because the logmanager is loaded parent first, the transformations are not being applied.

Hm, so even if we detect similar cases and omit the class from monitoring the error appears in the dev mode if a interceptor binding is explicitly declared. Maybe we should try to detect the problematic classes and fail the build?

This should be possible and is expected use of the library, and works without any problems with Quarkus 1.12.x.

Since 1.13 the container monitors business method invocations in the development mode by default. You can disable monitoring via quarkus.arc.dev-mode.monitoring-enabled=false and the error should go away because your bean is normally not intercepted.

Hm, maybe we should document this somewhere?

Yes, we can a paragraph to the Development Mode section. I can send a PR later today...

[geoand]

Hm, IIRC we try to remove the final modifier from the intercepted methods. @geoand It was originally implemented by you ;-). Could it be a problem because jboss logmanager classes are loaded by CL that does not support transformation?

Of course I did... :(.
So yeah, because the logmanager is loaded parent first, the transformations are not being applied.

Hm, so even if we detect similar cases and omit the class from monitoring the error appears in the dev mode if a interceptor binding is explicitly declared. Maybe we should try to detect the problematic classes and fail the build?

Yeah. I think there are 2 things here:

1. When there is no explicit interceptor binding created by the user, we should just simply not perform the transformation if any class in the class hierarchy is loaded parent first
2. If there is an explicit interceptor binding created by the user and any class in the class hierarchy is loaded parent first, we should fail the build

[mkouba]

any class in the class hierarchy is loaded parent first

Can we detect this somehow?

[pegrc]

Since 1.13 the container monitors business method invocations in the development mode by default. You can disable monitoring via quarkus.arc.dev-mode.monitoring-enabled=false and the error should go away because your bean is normally not intercepted.

Thanks, we'll give this a go for now :) and thanks everyone for looking into it

[geoand]

any class in the class hierarchy is loaded parent first

Can we detect this somehow?

For example, when checking at

quarkus/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Methods.java

Line 221 in d4dd21b

.addAll(addInterceptedMethodCandidates(beanDeployment, superClassInfo, candidates,

it would be something like:

ClassLoader tccl = Thread.currentThread().getContextClassLoader();
boolean isParentFirst = !tccl.loadClass(superClassInfo.name().toString()).getClassLoader().equals(tccl)

But we could likely loosen the access restrictions on QuarkusClassLoader and provide the ability to check if a class is parent first thus avoid loading it.
This would likely be a better solution.

[geoand]

#16617 is an implementation of the above idea

[mkouba]

@geoand Do you happen to know the status of this issue? ;-)

[geoand]

I don't think we ever utilized the method I added in #16617

[mkouba]

I'm going to close this issue. I don't think it makes sense to implement the parent first check to solve a corner case like this (bean class extends a jboss logging class). FYI the monitoring feature in the dev mode will be disabled by default in 2.14+ so you should not see the problem unless you add an interceptor binding to the ImplementedService bean or use quarkus.arc.dev-mode.monitoring-enabled=true.