Unexpected beans added to the index and affecting restarting the application

[pedroigor]

Describe the bug

I did not manage to come up with a reproducer using a regular Quarkus application. But the issue happens if you try out this branch https://github.com/pedroigor/keycloak/tree/tmp-reactive.

That branch is about switching from Resteasy Classic to Reactive in Keycloak. The issue does not happen if using Resteasy Classic and I'm not sure what is the relation, if any.

Basically, when reloading the application in dev mode or when running tests using the QuarkusMainTestExtension, reloading the application fails with this error:

org.junit.jupiter.api.extension.ParameterResolutionException: Failed to resolve parameter [io.quarkus.test.junit.main.LaunchResult arg0] in method [void org.keycloak.it.cli.StartCommandTest.testHttpEnabled(io.quarkus.test.junit.main.LaunchResult)]: java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
    [error]: Build step io.quarkus.arc.deployment.ArcProcessor#validate threw an exception: javax.enterprise.inject.spi.DeploymentException: Found 8 deployment problems: 
[1] Ambiguous dependencies for type javax.transaction.TransactionManager and qualifiers [@Default]
    - java member: io.quarkus.agroal.runtime.DataSources():transactionManager
    - declared on CLASS bean [types=[io.quarkus.agroal.runtime.DataSources, java.lang.Object], qualifiers=[@Default, @Any], target=io.quarkus.agroal.runtime.DataSources]
    - available beans:
        - CLASS bean [types=[java.io.Serializable, javax.transaction.TransactionManager, com.arjuna.ats.jta.cdi.NarayanaTransactionManager, com.arjuna.ats.jta.cdi.DelegatingTransactionManager, java.lang.Object], qualifiers=[@Default, @Any], target=com.arjuna.ats.jta.cdi.NarayanaTransactionManager]
        - CLASS bean [types=[javax.transaction.TransactionManager, java.io.Serializable, io.quarkus.narayana.jta.runtime.CDIDelegatingTransactionManager, java.lang.Object], qualifiers=[@Default, @Any], target=io.quarkus.narayana.jta.runtime.CDIDelegatingTransactionManager]

The first start of the application works fine and only CDIDelegatingTransactionManager is available to resolve TransactionManager bean types. However, reloading the application causes the
com.arjuna.ats.jta.cdi.NarayanaTransactionManager to also be recognized as a potential resolver hence failing with that error.

If we remove the com.arjuna.ats.jta.cdi.NarayanaTransactionManager at build time from our extension, the issue does not happen:

buildTimeConditionBuildItemBuildProducer.produce(new BuildTimeConditionBuildItem(index.getIndex().getClassByName(DotName.createSimple("com.arjuna.ats.jta.cdi.NarayanaTransactionManager")), false));`

Expected behavior

Being able to reload the application without introducing beans that otherwise should be excluded.

Actual behavior

Failing to reload the application as per the description.

How to Reproduce?

• Checkout this branch https://github.com/pedroigor/keycloak/tree/tmp-reactive-bean-issue
• mvn -Pdistribution -DskipTests clean install
• cd quarkus/server
• mvn quarkus:dev -Dquarkus.args=start-dev
• After starting Keycloak in dev mode, force a restart (type s)

Output of uname -a or ver

Linux fedora 6.0.9-200.fc36.x86_64 #1 SMP PREEMPT_DYNAMIC Wed Nov 16 17:50:45 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

Output of java -version

openjdk version "18.0.2.1" 2022-08-18 OpenJDK Runtime Environment (build 18.0.2.1+1-1) OpenJDK 64-Bit Server VM (build 18.0.2.1+1-1, mixed mode, sharing)

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.14.1.Final

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

No response

Additional information

No response

[pedroigor]

After talking with @stuartwdouglas, he mentioned that a possible cause is:

There is a thing called io.quarkus.deployment.index.PersistentClassIndex that stores the indexed classes between restarts to make hot reload faster
io.quarkus.arc.processor.BeanArchives#buildBeanArchiveIndex is (incorrectly) treating everything in PersistentClassIndex as a bean
But ReflectiveHeirachyBuildItem can result in additional stuff being added to PersistentClassIndex, including NarayanaTransactionManager if TransactionManager is involved anywhere (as this will index all implementors)
So on the second restart NarayanaTransactionManager is a bean

[quarkus-bot]

/cc @manovotn, @mkouba

[mkouba]

So the PersistentClassIndex got introduced in this pull request as an optimization for ArC where any class (not only but mostly from JDK) computed by the io.quarkus.arc.processor.BeanArchives.IndexWrapper was kept during restarts in the dev mode. In order to become a bean a class from this PersistentClassIndex must be annotated with a bean defining annotation (e.g. scope).

A few of months later it went into the core and got reused by the io.quarkus.deployment.index.IndexWrapper.

Basically, any client of BeanArchiveIndexBuildItem.getIndex() and CombinedIndexBuildItem.getComputingIndex() can add something to the persistent class index via IndexView#getClassByName().

Now, it seems that the com.arjuna.ats.jta.cdi.NarayanaTransactionManager is indexed druing the bean discovery (but after the BeanDeployment builds the set of known classes) and after the restart it becomes a known class and since it's annotated with @ApplicationScoped it becomes a bean and thus ambiguous dependencies.

I have no idea why it's indexed - I only found that it's indexed via BeanArchiveIndexBuildItem.getIndex() - but I think that the PersistentClassIndex should only retain JDK classes (which was probably the original intent) because it's not safe to retain all computed results.

CC @stuartwdouglas @manovotn @Ladicek

[mkouba]

By the way, the narayana-jta-5.13.1.Alpha1.jar contains the Jandex index so it should be part of the CombinedIndexBuildItem.getIndex() but it's ignored when building the bean archive because it does not contain beans.xml but a CDI portable extension and therefore is not considered an implicit CDI bean archive.

[pedroigor]

@mkouba Thanks. For now, I'm excluding NarayanaTransactionManager programmatically from our extension. Once the issue is fixed I'll remove this code.

Thanks for looking at it.

Out of curiosity, why it does not happen when using resteasy classic? Is there any relation you can think of? I can re-run my tests, but if I just revert our extension to use resteasy classic, everything works fine.

[mkouba]

Thanks. For now, I'm excluding NarayanaTransactionManager programmatically from our extension. Once the issue is fixed I'll remove this code.

Note that BuildTimeConditionBuildItem was not intended to be used to exclude a type from discovery. ExcludedTypeBuildItem is a more convenient build item for this task. Alternatively, you can use the quarkus.arc.exclude-types config property.

[mkouba]

Out of curiosity, why it does not happen when using resteasy classic? Is there any relation you can think of? I can re-run my tests, but if I just revert our extension to use resteasy classic, everything works fine.

Stuart was right that the NarayanaTransactionManager is indexed because of a ReflectiveHierarchyBuildItem, most likely produced for the javax.transaction.TransactionManager; this includes indexing of all known implementors/subclasses. And as I mentioned in #29575 (comment) - the NarayanaTransactionManager is included in the CombinedIndexBuildItem.

[mkouba]

#29641 should solve the problem...