Many "Failed to index" warnings since 3.9.1

[famod]

Describe the bug

Since 3.9.1 I'm getting the following warnings when building my app or running dev mode:

[WARNING] [io.quarkus.deployment.index.IndexWrapper] Failed to index org.springframework.context.support.AbstractApplicationContext: Class does not exist in ClassLoader QuarkusClassLoader:Deployment Class Loader: PROD for myapp-5.0.1.local-dev1@62d7d954
[WARNING] [io.quarkus.deployment.index.IndexWrapper] Failed to index com.sun.xml.fastinfoset.stax.StAXDocumentParser: Class does not exist in ClassLoader QuarkusClassLoader:Deployment Class Loader: PROD for myapp-5.0.1.local-dev1@62d7d954
[WARNING] [io.quarkus.deployment.index.IndexWrapper] Failed to index com.sun.xml.fastinfoset.stax.StAXDocumentSerializer: Class does not exist in ClassLoader QuarkusClassLoader:Deployment Class Loader: PROD for myapp-5.0.1.local-dev1@62d7d954
[WARNING] [io.quarkus.deployment.index.IndexWrapper] Failed to index org.springframework.context.ConfigurableApplicationContext: Class does not exist in ClassLoader QuarkusClassLoader:Deployment Class Loader: PROD for myapp-5.0.1.local-dev1@62d7d954
[WARNING] [io.quarkus.deployment.index.IndexWrapper] Failed to index io.mashona.logwriting.ArrayStore: Class does not exist in ClassLoader QuarkusClassLoader:Deployment Class Loader: PROD for myapp-5.0.1.local-dev1@62d7d954
[WARNING] [io.quarkus.deployment.index.IndexWrapper] Failed to index org.apache.activemq.artemis.core.journal.RecordInfo: Class does not exist in ClassLoader QuarkusClassLoader:Deployment Class Loader: PROD for myapp-5.0.1.local-dev1@62d7d954
[WARNING] [io.quarkus.deployment.index.IndexWrapper] Failed to index org.apache.activemq.artemis.core.journal.Journal: Class does not exist in ClassLoader QuarkusClassLoader:Deployment Class Loader: PROD for myapp-5.0.1.local-dev1@62d7d954
[WARNING] [io.quarkus.deployment.index.IndexWrapper] Failed to index jakarta.jms.XAConnection: Class does not exist in ClassLoader QuarkusClassLoader:Deployment Class Loader: PROD for myapp-5.0.1.local-dev1@62d7d954
[WARNING] [io.quarkus.deployment.index.IndexWrapper] Failed to index jakarta.jms.XASession: Class does not exist in ClassLoader QuarkusClassLoader:Deployment Class Loader: PROD for myapp-5.0.1.local-dev1@62d7d954
[WARNING] [io.quarkus.deployment.index.IndexWrapper] Failed to index jakarta.jms.XAConnectionFactory: Class does not exist in ClassLoader QuarkusClassLoader:Deployment Class Loader: PROD for myapp-5.0.1.local-dev1@62d7d954
[WARNING] [io.quarkus.deployment.index.IndexWrapper] Failed to index jakarta.jms.Connection: Class does not exist in ClassLoader QuarkusClassLoader:Deployment Class Loader: PROD for myapp-5.0.1.local-dev1@62d7d954

git bisect says #39564 is to blame. /cc @michalvavrik

Expected behavior

No such warnings (as in 3.9.0 and before)

Actual behavior

Warnings since 3.9.1

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

17.0.11

Quarkus version or git rev

3.9.4

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

Maven 3.9.6

Additional information

The respective Maven module doesn't have any sources, but it has dependencies to almost all other modules in the repo.
Interestingly, another module in the repo (which builds a different "flavor" of the app) doesn't have this issue, but it also has fewer dependencies (but it does contain some sources).

[quarkus-bot]

/cc @FroMage (resteasy-reactive), @geoand (resteasy-reactive), @stuartwdouglas (resteasy-reactive)

[famod]

@vavr I wanted to link @michalvavrik instead. I've correct that, sorry for the ping.

[gsmet]

Any chance we could have a small reproducer? (You knew it was coming!)

[michalvavrik]

Algorithm used in #39564 have to handle field class types that can have various subclasses and so on. I was thinking that we could just give up on detection in some classes if it will be too much (make them included to security serializer implicitly). However then I added a caching of this detection so I didn't think it was necessary.

Problem is that sometimes you can even have endpoint returning Object and I think if actual class returned from the endpoint is class with secured field, it should be secured as well. The case with Object we can simplify, but there can be others like this. That's why it is complex detection. If you provide a reproducer, I'd like to dig in.

[gsmet]

Do we really want to index additional classes though? For me if the class is not in the Jandex index, I would expect things to get ignored (similarly to JAX-RS resources will be ignored)

I'm a bit worried about the Object case you mention, what do you do in this case? You scan the whole classpath looking for potential @SecureField?

[michalvavrik]

Do we really want to index additional classes though? For me if the class is not in the Jandex index, I would expect things to get ignored (similarly to JAX-RS resources will be ignored)

I am sorry, I don't know indexes as well. IIRC I just used index that was already used for it, I just changed detection algorithm.

I'm a bit worried about the Object case you mention, what do you do in this case? You scan the whole classpath looking for potential @SecureField?

The algorithm that is in place must be able to detect that SomeType has a subtype with a field that is secured. For the Object itself I think condition can be vastly simplified: when there is at least one annotation instance with SecureField and Object is detected, answer is always that SecureField has been detected for that endpoint. Sadly I didn't think about that before. If it is really the Object (which we are just guessing) that fix is easy. I'll simplify it anyway now that I know about that.

[gsmet]

I haven't digged too much but I wonder if things shouldn't be done the opposite way i.e. scan the index for secure fields and determine the set of interfaces/classes that could be impacted.

That way we would scan the index only once.
Typically, I'm a bit worried about how well the current approach would scale when you have a lot of endpoints.

[michalvavrik]

I haven't digged too much but I wonder if things shouldn't be done the opposite way i.e. scan the index for secure fields and determine the set of interfaces/classes that could be impacted.

If I detect field annotated with SecureField on the class that's superclass is a field class of the class that is implementation of the interface returned from the endpoint, then I don't think we can go from that annotation instance. Is it supported by Jandex, please? It sounds really good, but I didn't find such an option.

That way we would scan the index only once.
Typically, I'm a bit worried about how well the current approach would scale when you have a lot of endpoints.

My understanding is that if I keep in hashmap "scanned" types, than I don't do it twice and that map is kept for all the endpoints. If this should affect build time or memory recognizably, then it needs to by fixed.

cc @geoand

I tried to improve the algorithm to detect more cases, this algorithm is not perfect because it can be very complex with generics, wildcards etc.

[gsmet]

Yeah so first you would have to find classes with @SecuredField.

Then you would have to get a list of all classes potentially impacted by this. Interfaces/superclasses/subclasses of the class are easy.
I'm not entirely sure how easy it would be to get the occurrences in other classes.

Jandex has a IndexView#getKnownUsers() method that maybe is what we want. But I think you should clarify first what you actually want to look at and also why exactly are you trying to detect the @SecureField so broadly?
I don't have much context tbh.

Once we have clarified what we want to do, we can discuss with @Ladicek what our options with Jandex are.

[michalvavrik]

Yeah so first you would have to find classes with @SecuredField.

Then you would have to get a list of all classes potentially impacted by this. Interfaces/superclasses/subclasses of the class are easy. I'm not entirely sure how easy it would be to get the occurrences in other classes.

Jandex has a IndexView#getKnownUsers() method that maybe is what we want.

annotation instance -> field type -> getKnownUsers and recursion up to the object until we find what can be an endpoint can work. Fields annotated with SecureField can be String and int and so on. I cannot tell if such algorithm is really better for the performance. However if you are convinced or there is an issue with the algorithm in place, I am happy to try it.

But I think you should clarify first what you actually want to look at and also why exactly are you trying to detect the @SecureField so broadly? I don't have much context tbh.

I was fixing #39513 and I thought it would be lame to just fix one case out of many. Right now behavior of the SecureField is not based on documented definition (in other words - I did not find documented definition to follow), so I believe it was better to improve it, but users must always test it. I can't tell what is expected behavior. @geoand can define it and then, we can alter if there is an issue with current behavior.

Once we have clarified what we want to do, we can discuss with @Ladicek what our options with Jandex are.

Alright, thank you.

[Ladicek]

I'm not sure if I understand the issue correctly, but I don't think a single scan over the index will ever be enough. Regardless of whether you start with the types that can be returned, or with the types that have a @SecureField annotation, you will always have to iterate until nothing new is found (or you reach a defined limit).

I don't know what would be the best course of action here, but I think it would be good to filter out some fields (such as static fields or @JsonIgnore fields) early on. That should have a decent chance of getting rid of these warnings.

[michalvavrik]

Hello,

I don't want to forget about this, @famod is there a chance that you can provide a reproducer without too much of a work? Now I have mentioned that keycloak/keycloak#28947 exists so I think I can try to use the Keycloak project. I'll try it later this week.

I'm not convinced that approach suggested by @gsmet is more performant due to how many usages primitive data types will have. I'll try @Ladicek suggestion first and see what the results are.

Thanks

[gsmet]

Fields annotated with SecureField can be String and int and so on.

Not sure how this related to what I wrote. You don't care about the type of the field but the type of the enclosing class AFAIU.

Now apparently getKnownUser() doesn't work in the case of fields. @Ladicek can you confirm this?

[michalvavrik]

Fields annotated with SecureField can be String and int and so on.

Not sure how this related to what I wrote. You don't care about the type of the field but the type of the enclosing class AFAIU.

Now apparently getKnownUser() doesn't work in the case of fields. @Ladicek can you confirm this?

The way I understood your suggestion is to lookup annotation instances, check field type and get known users. Sure, if it will start working with fields, it's better solution.

[Ladicek]

Now apparently getKnownUser() doesn't work in the case of fields. @Ladicek can you confirm this?

No, it's a lot worse :-) The getKnownUsers() method only considers a class as used in another class when it occurs in the list of class references in the other class's constant pool. This PR smallrye/jandex#354 should improve it.

[gsmet]

The way I understood your suggestion is to lookup annotation instances, check field type and get known users. Sure, if it will start working with fields, it's better solution.

Oh so there was a misunderstanding. For me, you start at the field but get the users of the enclosing class (to start building a tree of how it is used).
FWIW, I had a similar requirement for the config work we were doing with Roberto (but with methods return type) so I think we should probably have a way to determine the list of paths leading to a specific type/field/method.

[michalvavrik]

Oh so there was a misunderstanding. For me, you start at the field but get the users of the enclosing class (to start building a tree of how it is used).
FWIW, I had a similar requirement for the config work we were doing with Roberto (but with methods return type) so I think we should probably have a way to determine the list of paths leading to a specific type/field/method.

Sounds good @gsmet , indeed that's better. Thanks. IIUC it will require @Ladicek PR.

I'll think about temporary fixes till then when I debug this with the Keycloak reproducer.

[gsmet]

Now, as mentioned by @Ladicek, we might have to apply the same exclusions we apply to when we index ReflectiveHierarchy. IIRC we have a specific exclusion for JAX-RS return types already which might be slightly related to what you're doing.

[michalvavrik]

Alright, thanks for all the help. I've opened #40448 and also detected case that didn't work (#40447). Tested it with Keycloak project on main branch (running quarkus/server in dev mode after switch from resteasy-reactive to quarkus-rest) and this fixes their issue.

We can follow-up when Jandex gets planned improvements.

[famod]

Fix confirmed in 3.10.1, thanks!

PS: Sorry for the lack of feedback, I've been on PTO for over two weeks.

[michalvavrik]

Fix confirmed in 3.10.1, thanks!

PS: Sorry for the lack of feedback, I've been on PTO for over two weeks.

np, thank you for the confirmation

[woprandi]

I got these warnings on 3.8.4 (with latest Keycloak) so I'm surprised about the title

[michalvavrik]

I got these warnings on 3.8.4 (with latest Keycloak) so I'm surprised about the title

Hey @woprandi , I agree it is not a straighforward, but 3.9.1 was released on March 27 and 3.8.4 on April 17, therefore it makes sense users first experienced it in 3.9.1.

[woprandi]

@michalvavrik Ok I see thanks

[gsmet]

@famod can you test with 3.10.1 and make sure things work properly for you?

[famod]

@gsmet

@famod can you test with 3.10.1 and make sure things work properly for you?

That's what I was trying to say with

Fix confirmed in 3.10.1

Meaning with 3.10.1 I don't see those warnings anymore and haven't spotted any regressions thus far.

[mabartos]

Typically, I'm a bit worried about how well the current approach would scale when you have a lot of endpoints.

If this should affect build time or memory recognizably, then it needs to be fixed.

@michalvavrik @gsmet The scalability is not very good, I'd say. After merging this PR #39564, the augmentation time significantly increased for the Keycloak project. Please see keycloak/keycloak#29579.

The situation for a "plain" Keycloak might be kinda acceptable, but we support extending the Keycloak functionality by adding custom providers (JARs) for handling custom REST endpoints, and it's much worse.

As seen in keycloak/keycloak#29579, the augmentation time for the build step increased from ~17ms to ~800ms without any additional JARs. With the provided JARs, it increased from ~20ms to ~44500ms.

@cwelch-rh might provide more information on this.

@cwelch-rh Might be good to create a separate issue for this.

[michalvavrik]

@mabartos I expect after #40448 times for Keycloak itself are better than they were even before #39564. That is situation has improved against previous state now. Can you give it a try with Quarkus 3.10.1, please?

As seen in keycloak/keycloak#29579, the augmentation time for the build step increased from ~17ms to ~800ms without any additional JARs. With the provided JARs, it increased from ~20ms to ~44500ms.

The keycloak/keycloak#29579 steps to reproduce says The increase in time can be seen without providers, but it's more dramatic with ours.. I don't know how to add providers. It would be interesting for me to experiment with these horrific numbers against 3.8.4 as I would add my custom secure fields, but I would need more specific steps to reproduce, probably added to the KC issue.

[michalvavrik]

@Ladicek I read https://smallrye.io/blog/jandex-3-2-0/ Index.getKnownUsers() passage and I have one question, is a class considered as a user of another class when the parent class or interface of the other class is present in any of its members, please?

[Ladicek]

@michalvavrik No, the class will only be considered as a user of the parent class.

[michalvavrik]

@michalvavrik No, the class will only be considered as a user of the parent class.

thank you

[gsmet]

@mabartos @cwelch-rh any chance we could have some profiling data? We provide some information about how to profile using asyncprofiler here: https://github.com/quarkusio/quarkus/blob/main/TROUBLESHOOTING.md .

Also, yes, please create a new issue.

[gsmet]

And I agree with @michalvavrik that it would be interesting to have a datapoint with 3.10.1. We might have to backport the subsequent patch to 3.8 if it improves things for you significantly.

[gsmet]

@mabartos @cwelch-rh if you want fixes to be included in the next 3.8, we will need to move fast. So could you have a look at the comments above ^? Thanks!

[cwelch-rh]

I was able to test with 3.10.1, the augmentation time looks good to me. These times are about what we expect.

With providers:

[io.qua.dep.QuarkusAugmentor] (main) Quarkus augmentation completed in 8742ms

Without:

[io.qua.dep.QuarkusAugmentor] (main) Quarkus augmentation completed in 6092ms