Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid calling ResteasyProviderFactory.getInstance() when popping #41431

Merged
merged 1 commit into from
Jun 26, 2024

Conversation

gsmet
Copy link
Member

@gsmet gsmet commented Jun 25, 2024

When popping provider factory, what we actually want is pop it if something has been pushed.
It is better handled by setting a boolean if something has been pushed.

This allows class loading issues when the class loader has been closed and the request is still processing.

Related to #41233

When popping provider factory, what we actually want is pop it if
something has been pushed.
It is better handled by setting a boolean if something has been pushed.

This allows class loading issues when the class loader has been closed
and the request is still processing.

Related to quarkusio#41233
@geoand
Copy link
Contributor

geoand commented Jun 25, 2024

I can't say I understand this one. I'll look at it tomorrow

Copy link

quarkus-bot bot commented Jun 25, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 4fea558.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.


Flaky tests - Develocity

⚙️ JVM Tests - JDK 21

📦 integration-tests/reactive-messaging-kafka

io.quarkus.it.kafka.KafkaConnectorTest.testFruits - History

  • Assertion condition defined as a Lambda expression in io.quarkus.it.kafka.KafkaConnectorTest expected: <6> but was: <5> within 10 seconds. - org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: Assertion condition defined as a Lambda expression in io.quarkus.it.kafka.KafkaConnectorTest expected: <6> but was: <5> within 10 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:31)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1006)
	at org.awaitility.core.ConditionFactory.untilAsserted(ConditionFactory.java:790)
	at io.quarkus.it.kafka.KafkaConnectorTest.testFruits(KafkaConnectorTest.java:63)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)

@gsmet
Copy link
Member Author

gsmet commented Jun 25, 2024

It's related to this issue: #41233

And more specifically to this stacktrace that I got with my code dumping the stack when a CL is accessed while closed:

2024-06-16T15:34:06.3031526Z Caused by: java.lang.IllegalStateException: Class loader Quarkus Runtime ClassLoader: DEV for testChunkedLargeRequest() (QuarkusDevModeTest) restart no:0 has been closed and may not be accessed anymore
2024-06-16T15:34:06.3034400Z 	at io.quarkus.bootstrap.classloading.QuarkusClassLoader.ensureOpen(QuarkusClassLoader.java:727)
2024-06-16T15:34:06.3036325Z 	at io.quarkus.bootstrap.classloading.QuarkusClassLoader.getResources(QuarkusClassLoader.java:226)
2024-06-16T15:34:06.3038011Z 	at java.base/java.util.ServiceLoader$LazyClassPathLookupIterator.nextProviderClass(ServiceLoader.java:1203)
2024-06-16T15:34:06.3176043Z 	at java.base/java.util.ServiceLoader$LazyClassPathLookupIterator.hasNextService(ServiceLoader.java:1228)
2024-06-16T15:34:06.3177925Z 	at java.base/java.util.ServiceLoader$LazyClassPathLookupIterator.hasNext(ServiceLoader.java:1273)
2024-06-16T15:34:06.3179227Z 	at java.base/java.util.ServiceLoader$2.hasNext(ServiceLoader.java:1309)
2024-06-16T15:34:06.3180271Z 	at java.base/java.util.ServiceLoader$3.hasNext(ServiceLoader.java:1393)
2024-06-16T15:34:06.3181646Z 	at io.smallrye.config.SmallRyeConfigProviderResolver.getFactoryFor(SmallRyeConfigProviderResolver.java:102)
2024-06-16T15:34:06.3183298Z 	at io.smallrye.config.SmallRyeConfigProviderResolver.getConfig(SmallRyeConfigProviderResolver.java:78)
2024-06-16T15:34:06.3184881Z 	at io.smallrye.config.SmallRyeConfigProviderResolver.getConfig(SmallRyeConfigProviderResolver.java:66)
2024-06-16T15:34:06.3186265Z 	at org.eclipse.microprofile.config.ConfigProvider.getConfig(ConfigProvider.java:85)
2024-06-16T15:34:06.3188279Z 	at org.jboss.resteasy.microprofile.config.ConfigConfiguration.<init>(ConfigConfiguration.java:36)
2024-06-16T15:34:06.3189993Z 	at org.jboss.resteasy.microprofile.config.ConfigConfigurationFactory.getConfiguration(ConfigConfigurationFactory.java:32)
2024-06-16T15:34:06.3191645Z 	at org.jboss.resteasy.plugins.providers.RegisterBuiltin$5.run(RegisterBuiltin.java:178)
2024-06-16T15:34:06.3193059Z 	at org.jboss.resteasy.plugins.providers.RegisterBuiltin$5.run(RegisterBuiltin.java:175)
2024-06-16T15:34:06.3194411Z 	at java.base/java.security.AccessController.doPrivileged(AccessController.java:318)
2024-06-16T15:34:06.3195855Z 	at org.jboss.resteasy.plugins.providers.RegisterBuiltin.isGZipEnabled(RegisterBuiltin.java:175)
2024-06-16T15:34:06.3197355Z 	at org.jboss.resteasy.plugins.providers.RegisterBuiltin.<clinit>(RegisterBuiltin.java:37)

I tracked where this was called and it was in the finally clause. Somehow, the CL was closed then and getting the instance would reload some classes to clinit RegisterBuiltin again.

Now I'm not entirely sure how the CL gets closed by the time we arrive there but in any case, what we are doing is not ideal given what we really want is to pop when we have pushed. The test was copied but really we just want to have a boolean that indicates we have pushed something and we need to pop it.

Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, makes sense!

@gsmet gsmet merged commit d8ec6f1 into quarkusio:main Jun 26, 2024
50 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.13 - main milestone Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants