quarkus
quarkus copied to clipboard
quarkusio
Reinstate changes to use JBoss serializer instead of xstream
This reinstates #40601. See discussion in https://github.com/quarkusio/quarkus/pull/40880. This PR reintroduces @dmlloyd's lovely #40601, now that JUnit has been updated to a version which is compatible with #40601, 5.10.3.
It also includes a fix for a functional issue in the change that we couldn't see when it was blocked by JUnit problems (and earlier, by a lack of test coverage).
Thanks for your pull request!
The title of your pull request does not follow our editorial rules. Could you have a look?
- title should not contain an issue number (use
Fix #1234in the description instead)
This message is automatically generated by a bot.
![quarkus-bot[bot] avatar](https://avatars.githubusercontent.com/in/90234?v=4 "Published by a pub.dev verified publisher"){kind=small}
:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.
Status for workflow Quarkus CI
This is the status report for running Quarkus CI on commit a712fe929a761e238283cd5e870009544f88f6d4.
Failing Jobs
| Status | Name | Step | Failures | Logs | Raw logs | Build scan |
|---|---|---|---|---|---|---|
| ✖ | Initial JDK 17 Build | Build |
:warning: Check → | Logs | Raw logs | :construction: |
https://github.com/junit-team/junit5/releases/tag/r5.10.3 🎉
🎉 🎉
Will update to that level in this PR and see how CI goes (did we get the backport?)
The test failure is definitely related:
2024-06-27T22:31:00.7017409Z Press [e] to edit command line args (currently ''), [r] to re-run, [o] Toggle test output, [h] for more options>
2024-06-27T22:31:00.7019039Z 2024-06-27 22:31:00,573 ERROR [io.qua.test] (Test runner thread) ==================== TEST REPORT #1 ====================
2024-06-27T22:31:00.7020830Z 2024-06-27 22:31:00,574 ERROR [io.qua.test] (Test runner thread) Test TemplatedQuarkusTest#[@io.quarkus.test.junit.QuarkusTest()] failed
2024-06-27T22:31:00.7022050Z : java.lang.ExceptionInInitializerError
2024-06-27T22:31:00.7022979Z at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
2024-06-27T22:31:00.7024208Z at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:480)
2024-06-27T22:31:00.7025819Z at org.jboss.marshalling.reflect.SerializableClass.invokeConstructorNoException(SerializableClass.java:404)
2024-06-27T22:31:00.7027534Z at org.jboss.marshalling.reflect.SerializableClass.callNonInitConstructor(SerializableClass.java:372)
2024-06-27T22:31:00.7029013Z at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:309)
2024-06-27T22:31:00.7030301Z at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:132)
2024-06-27T22:31:00.7031660Z at org.jboss.marshalling.cloner.SerializingCloner.cloneFields(SerializingCloner.java:452)
2024-06-27T22:31:00.7033155Z at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:372)
2024-06-27T22:31:00.7034614Z at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:315)
2024-06-27T22:31:00.7036045Z at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:132)
2024-06-27T22:31:00.7037450Z at org.jboss.marshalling.cloner.SerializingCloner$StepObjectInput.doReadObject(SerializingCloner.java:930)
2024-06-27T22:31:00.7038955Z at org.jboss.marshalling.AbstractObjectInput.readObject(AbstractObjectInput.java:41)
2024-06-27T22:31:00.7040462Z at org.jboss.marshalling.MarshallerObjectInputStream.readObjectOverride(MarshallerObjectInputStream.java:53)
2024-06-27T22:31:00.7042912Z at java.base/java.io.ObjectInputStream.readObject(ObjectInputStream.java:500)
2024-06-27T22:31:00.7044111Z at java.base/java.io.ObjectInputStream.readObject(ObjectInputStream.java:467)
2024-06-27T22:31:00.7045326Z at java.base/java.util.ArrayList.readObject(ArrayList.java:899)
2024-06-27T22:31:00.7046513Z at org.jboss.marshalling.reflect.JDKSpecific$SerMethods.callReadObject(JDKSpecific.java:171)
2024-06-27T22:31:00.7047969Z at org.jboss.marshalling.reflect.SerializableClass.callReadObject(SerializableClass.java:252)
2024-06-27T22:31:00.7049590Z at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:366)
2024-06-27T22:31:00.7051145Z at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:315)
2024-06-27T22:31:00.7052507Z at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:132)
2024-06-27T22:31:00.7054069Z at org.jboss.marshalling.cloner.SerializingCloner.cloneFields(SerializingCloner.java:452)
2024-06-27T22:31:00.7055734Z at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:372)
2024-06-27T22:31:00.7057225Z at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:315)
2024-06-27T22:31:00.7058675Z at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:132)
2024-06-27T22:31:00.7060297Z at org.jboss.marshalling.cloner.SerializingCloner$StepObjectInput.doReadObject(SerializingCloner.java:930)
2024-06-27T22:31:00.7061824Z at org.jboss.marshalling.AbstractObjectInput.readObject(AbstractObjectInput.java:41)
2024-06-27T22:31:00.7063427Z at org.jboss.marshalling.MarshallerObjectInputStream.readObjectOverride(MarshallerObjectInputStream.java:53)
2024-06-27T22:31:00.7064929Z at java.base/java.io.ObjectInputStream.readObject(ObjectInputStream.java:500)
2024-06-27T22:31:00.7066255Z at java.base/java.io.ObjectInputStream.readObject(ObjectInputStream.java:467)
2024-06-27T22:31:00.7067387Z at java.base/java.util.ArrayList.readObject(ArrayList.java:899)
2024-06-27T22:31:00.7068559Z at org.jboss.marshalling.reflect.JDKSpecific$SerMethods.callReadObject(JDKSpecific.java:171)
2024-06-27T22:31:00.7070045Z at org.jboss.marshalling.reflect.SerializableClass.callReadObject(SerializableClass.java:252)
2024-06-27T22:31:00.7071659Z at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:366)
2024-06-27T22:31:00.7073166Z at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:315)
2024-06-27T22:31:00.7074498Z at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:132)
2024-06-27T22:31:00.7076010Z at org.jboss.marshalling.cloner.SerializingCloner.cloneFields(SerializingCloner.java:452)
2024-06-27T22:31:00.7077533Z at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:372)
2024-06-27T22:31:00.7079009Z at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:315)
2024-06-27T22:31:00.7080360Z at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:132)
2024-06-27T22:31:00.7081769Z at org.jboss.marshalling.cloner.SerializingCloner.cloneFields(SerializingCloner.java:452)
2024-06-27T22:31:00.7083329Z at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:372)
2024-06-27T22:31:00.7084856Z at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:315)
2024-06-27T22:31:00.7086329Z at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:132)
2024-06-27T22:31:00.7087714Z at org.jboss.marshalling.cloner.SerializingCloner.cloneFields(SerializingCloner.java:452)
2024-06-27T22:31:00.7089280Z at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:372)
2024-06-27T22:31:00.7090832Z at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:315)
2024-06-27T22:31:00.7092327Z at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:132)
2024-06-27T22:31:00.7093882Z at org.jboss.marshalling.cloner.SerializingCloner$StepObjectInput.doReadObject(SerializingCloner.java:930)
2024-06-27T22:31:00.7095617Z at org.jboss.marshalling.AbstractObjectInput.readObject(AbstractObjectInput.java:41)
2024-06-27T22:31:00.7097291Z at org.jboss.marshalling.MarshallerObjectInputStream.readObjectOverride(MarshallerObjectInputStream.java:53)
2024-06-27T22:31:00.7098833Z at java.base/java.io.ObjectInputStream.readObject(ObjectInputStream.java:500)
2024-06-27T22:31:00.7100050Z at java.base/java.io.ObjectInputStream.readObject(ObjectInputStream.java:467)
2024-06-27T22:31:00.7101144Z at java.base/java.util.ArrayList.readObject(ArrayList.java:899)
2024-06-27T22:31:00.7102344Z at org.jboss.marshalling.reflect.JDKSpecific$SerMethods.callReadObject(JDKSpecific.java:171)
2024-06-27T22:31:00.7103975Z at org.jboss.marshalling.reflect.SerializableClass.callReadObject(SerializableClass.java:252)
2024-06-27T22:31:00.7105627Z at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:366)
2024-06-27T22:31:00.7107059Z at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:315)
2024-06-27T22:31:00.7108415Z at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:132)
2024-06-27T22:31:00.7109774Z at org.jboss.marshalling.cloner.SerializingCloner.cloneFields(SerializingCloner.java:452)
2024-06-27T22:31:00.7111251Z at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:372)
2024-06-27T22:31:00.7112756Z at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:315)
2024-06-27T22:31:00.7114105Z at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:132)
2024-06-27T22:31:00.7115795Z at org.jboss.marshalling.cloner.SerializingCloner.cloneFields(SerializingCloner.java:452)
2024-06-27T22:31:00.7116808Z at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:372)
2024-06-27T22:31:00.7117659Z at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:315)
2024-06-27T22:31:00.7118421Z at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:132)
2024-06-27T22:31:00.7119301Z at org.jboss.marshalling.cloner.SerializingCloner$StepObjectInput.doReadObject(SerializingCloner.java:930)
2024-06-27T22:31:00.7120174Z at org.jboss.marshalling.AbstractObjectInput.readObject(AbstractObjectInput.java:41)
2024-06-27T22:31:00.7121079Z at org.jboss.marshalling.MarshallerObjectInputStream.readObjectOverride(MarshallerObjectInputStream.java:53)
2024-06-27T22:31:00.7121939Z at java.base/java.io.ObjectInputStream.readObject(ObjectInputStream.java:500)
2024-06-27T22:31:00.7122625Z at java.base/java.io.ObjectInputStream.readObject(ObjectInputStream.java:467)
2024-06-27T22:31:00.7123238Z at java.base/java.util.ArrayList.readObject(ArrayList.java:899)
2024-06-27T22:31:00.7123926Z at org.jboss.marshalling.reflect.JDKSpecific$SerMethods.callReadObject(JDKSpecific.java:171)
2024-06-27T22:31:00.7124765Z at org.jboss.marshalling.reflect.SerializableClass.callReadObject(SerializableClass.java:252)
2024-06-27T22:31:00.7125882Z at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:366)
2024-06-27T22:31:00.7126723Z at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:315)
2024-06-27T22:31:00.7127479Z at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:132)
2024-06-27T22:31:00.7128298Z at org.jboss.marshalling.cloner.SerializingCloner.cloneFields(SerializingCloner.java:452)
2024-06-27T22:31:00.7129176Z at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:372)
2024-06-27T22:31:00.7130016Z at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:315)
2024-06-27T22:31:00.7130878Z at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:132)
2024-06-27T22:31:00.7131670Z at org.jboss.marshalling.cloner.SerializingCloner.cloneFields(SerializingCloner.java:452)
2024-06-27T22:31:00.7132544Z at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:372)
2024-06-27T22:31:00.7133473Z at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:340)
2024-06-27T22:31:00.7134314Z at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:315)
2024-06-27T22:31:00.7135058Z at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:132)
2024-06-27T22:31:00.7136132Z at org.jboss.marshalling.cloner.SerializingCloner.cloneFields(SerializingCloner.java:452)
2024-06-27T22:31:00.7137094Z at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:372)
2024-06-27T22:31:00.7138091Z at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:340)
2024-06-27T22:31:00.7138927Z at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:315)
2024-06-27T22:31:00.7139665Z at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:132)
2024-06-27T22:31:00.7140494Z at org.jboss.marshalling.cloner.SerializingCloner.cloneFields(SerializingCloner.java:452)
2024-06-27T22:31:00.7141410Z at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:372)
2024-06-27T22:31:00.7142321Z at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:340)
2024-06-27T22:31:00.7143162Z at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:315)
2024-06-27T22:31:00.7143902Z at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:132)
2024-06-27T22:31:00.7144725Z at io.quarkus.test.junit.internal.NewSerializingDeepClone.clone(NewSerializingDeepClone.java:92)
2024-06-27T22:31:00.7145795Z at io.quarkus.test.junit.QuarkusTestExtension.runExtensionMethod(QuarkusTestExtension.java:965)
2024-06-27T22:31:00.7146668Z at io.quarkus.test.junit.QuarkusTestExtension.runExtensionMethod(QuarkusTestExtension.java:905)
2024-06-27T22:31:00.7147610Z at io.quarkus.test.junit.QuarkusTestExtension.interceptTestTemplateMethod(QuarkusTestExtension.java:865)
2024-06-27T22:31:00.7148462Z at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183)
2024-06-27T22:31:00.7149195Z at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
2024-06-27T22:31:00.7149941Z at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:179)
2024-06-27T22:31:00.7150655Z at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
2024-06-27T22:31:00.7151418Z at java.base/java.util.stream.Streams$StreamBuilderImpl.forEachRemaining(Streams.java:411)
2024-06-27T22:31:00.7152197Z at java.base/java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:762)
2024-06-27T22:31:00.7152943Z at java.base/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:276)
2024-06-27T22:31:00.7153738Z at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1625)
2024-06-27T22:31:00.7154506Z at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
2024-06-27T22:31:00.7155423Z at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
2024-06-27T22:31:00.7156217Z at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150)
2024-06-27T22:31:00.7157016Z at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173)
2024-06-27T22:31:00.7157794Z at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
2024-06-27T22:31:00.7158531Z at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
2024-06-27T22:31:00.7159208Z at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
2024-06-27T22:31:00.7159726Z at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
2024-06-27T22:31:00.7160257Z Caused by: java.util.NoSuchElementException: No value present
2024-06-27T22:31:00.7160740Z at java.base/java.util.Optional.get(Optional.java:143)
2024-06-27T22:31:00.7161386Z at io.quarkus.deployment.dev.testing.TestConsoleHandler.<clinit>(TestConsoleHandler.java:38)
2024-06-27T22:31:00.7161942Z ... 117 more
2024-06-27T22:31:00.7162066Z
2024-06-27T22:31:00.7162610Z 2024-06-27 22:31:00,582 ERROR [io.qua.test] (Test runner thread) >>>>>>>>>>>>>>>>>>>> Summary: <<<<<<<<<<<<<<<<<<<<
2024-06-27T22:31:00.7163302Z TemplatedQuarkusTest#[@io.quarkus.test.junit.QuarkusTest()] null
2024-06-27T22:31:00.7164100Z 2024-06-27 22:31:00,585 ERROR [io.qua.test] (Test runner thread) >>>>>>>>>>>>>>>>>>>> 1 TEST FAILED <<<<<<<<<<<<<<<<<<<<
2024-06-27T22:31:00.7164947Z 1 test failed (2 passing, 0 skipped), 3 tests were run in 2656ms. Tests completed at 22:31:00.
2024-06-27T22:31:08.6068071Z [ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 32.81 s <<< FAILURE! -- in io.quarkus.it.extension.it.TestTemplateDevModeIT
2024-06-27T22:31:08.6069789Z [ERROR] io.quarkus.it.extension.it.TestTemplateDevModeIT.testThatTheTestsPassed -- Time elapsed: 32.81 s <<< FAILURE!
2024-06-27T22:31:08.6070735Z org.opentest4j.AssertionFailedError: expected: <0> but was: <1>
2024-06-27T22:31:08.6071439Z at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
2024-06-27T22:31:08.6072865Z at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
2024-06-27T22:31:08.6074205Z at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
2024-06-27T22:31:08.6075342Z at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:166)
2024-06-27T22:31:08.6076036Z at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:161)
2024-06-27T22:31:08.6076689Z at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:632)
2024-06-27T22:31:08.6077505Z at io.quarkus.it.extension.it.TestTemplateDevModeIT.testThatTheTestsPassed(TestTemplateDevModeIT.java:59)
2024-06-27T22:31:08.6078281Z at java.base/java.lang.reflect.Method.invoke(Method.java:568)
2024-06-27T22:31:08.6078807Z at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
2024-06-27T22:31:08.6079312Z at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
Yeah, that test failure is the one we'd expect if @dmlloyd's fix hadn't been backported.
It's hard to tell if it's been backported or not. Confusing! https://github.com/junit-team/junit5/pull/3837 is the 5.10.x PR, which is still open. (https://github.com/junit-team/junit5/pull/3820 is the 5.11.x PR, which is merged.)
But ... if I check https://github.com/junit-team/junit5/commits/releases/5.10.x/, I can see 3820 in there, which would be consistent with them just doing the backport themselves.
So then the question is ... why is 5.10.3 + these changes regressing that test? :(
I've checked https://github.com/quarkusio/quarkus/pull/40773, and the stack trace is a bit different. Without 5.10.3, the stack trace is:
2024-05-22 11:53:10,847 ERROR [io.qua.test] (Test runner thread) ==================== TEST REPORT #1 ====================
2024-05-22 11:53:10,847 ERROR [io.qua.test] (Test runner thread) Test TemplatedQuarkusTest#[@io.quarkus.test.junit.QuarkusTest()] failed
: java.lang.NullPointerException: Cannot invoke "org.junit.platform.engine.UniqueId.toString()" because the return value of "org.junit.platform.launcher.TestIdentifier.access$800(org.junit.platform.launcher.TestIdentifier)" is null
So this is definitely a regression, and one that seems related to either 5.10.3 or #40601, but more investigation will be needed. We can apply 5.10.3 without #40601, so I'll try that first, just for diagnostics.
We can apply 5.10.3
We actually already have it in main. Dependabot did a PR that I merged
We can apply 5.10.3
We actually already have it in
main. Dependabot did a PR that I merged
Ah, I looked for the dependabot PR but didn't think to look at the closed ones. I should have remembered how fast you are. :)
OK, I'll confirm whether this affects Pact as well (seems likely), confirm I can reproduce locally, and see what I can see.
I should have remembered how fast you are. :)
Or basically that I am 2 hours ahead in my TZ :P
I confirm that this
- Is not intermittent, even though it kind of looks like it should be a timing-y, thread-y problem
- Affects the pact provider tests, so would be a regression that's visible externally. Anything using JUnit templated tests would have a problem.
The 'root symptom' is that RuntimeUpdatesProcessor.INSTANCE is null, but it's not obvious to me (yet) why anything in #40601 would affect that. On the other hand, test templates are one of the main paths where we exercise the serialisation (which is why the Pact extension gets issues raised against it about the xstream cloner not working on recent Java versions, etc)
Investigation continues.
Could it be that RuntimeUpdatesProcessor is getting duplicated onto both class loaders, and it's the "wrong one" being accessed? Or something along those lines?
Could it be that
RuntimeUpdatesProcessoris getting duplicated onto both class loaders, and it's the "wrong one" being accessed? Or something along those lines?
Yes, that's my thinking too. The whole "actually load test classes with the classloader we want to use to run them PR" would fix that issue, but we're clearly not going to wait for that, partly because it's been cooking for like a year, and partly because if/when it ever does drop, it would obsolete the changes in this PR anyway. :)
I wonder if there's a simple change in what classloader we pass in on this line that would help things:
https://github.com/quarkusio/quarkus/pull/40906/files#diff-462fa0cb443c21c9406c8b4daeb31500f20a4c9b46a0b039adcc93dee6a3bea4R352
Could it be that
RuntimeUpdatesProcessoris getting duplicated onto both class loaders, and it's the "wrong one" being accessed? Or something along those lines?Yes, that's my thinking too. The whole "actually load test classes with the classloader we want to use to run them PR" would fix that issue, but we're clearly not going to wait for that, partly because it's been cooking for like a year, and partly because if/when it ever does drop, it would obsolete the changes in this PR anyway. :)
I wonder if there's a simple change in what classloader we pass in on this line that would help things:
https://github.com/quarkusio/quarkus/pull/40906/files#diff-462fa0cb443c21c9406c8b4daeb31500f20a4c9b46a0b039adcc93dee6a3bea4R352
The first class loader argument (the source class loader) was (iirc) originally used as a test for the class being cloned, but it didn't end up being usable in that capacity. Now it's only used for cases where the actual class loader instance appears, and it's substituted with the target class loader. I don't recall if/why that is needed though; we might be able to just drop that functionality completely and get rid of the argument.
The second class loader is the loader used to load the actual replacement classes; the rule is roughly "if the class exists in the test loader, clone the object". But there are some classes we statically know we don't want replicated. Maybe RuntimeUpdatesProcessor is one of them?
It's a bit goofy that the deployment classes get duplicated too, but I guess that's necessary because they depend on the runtime dependencies.
Could it be that
RuntimeUpdatesProcessoris getting duplicated onto both class loaders, and it's the "wrong one" being accessed? Or something along those lines?Yes, that's my thinking too. The whole "actually load test classes with the classloader we want to use to run them PR" would fix that issue, but we're clearly not going to wait for that, partly because it's been cooking for like a year, and partly because if/when it ever does drop, it would obsolete the changes in this PR anyway. :)
This will be the best solution, when it arrives.
The first class loader argument (the source class loader) was (iirc) originally used as a test for the class being cloned, but it didn't end up being usable in that capacity. Now it's only used for cases where the actual class loader instance appears, and it's substituted with the target class loader. I don't recall if/why that is needed though; we might be able to just drop that functionality completely and get rid of the argument.
I made a crude attempt at this (just passing through the application class loader for both arguments), and I can confirm the first argument is being used, and has an effect. :) I think it might be worth pursuing getting rid of it, as a tidy-up, but it won't be trivial.
The second class loader is the loader used to load the actual replacement classes; the rule is roughly "if the class exists in the test loader, clone the object". But there are some classes we statically know we don't want replicated. Maybe
RuntimeUpdatesProcessoris one of them?
I think this is the right approach. I've got the affected test passing with a variation of this 🎉 🎉 🎉. Just not-replicating RuntimeUpdatesProcessor wasn't enough, so I went further and didn't replicate any Quarkus classes. I think there's probably a performance gain from not replicating them, and they're already in the 'right' classloader, so I don't think there's a functional reason to replicate them. I'm also fairly confident the previous implementation couldn't have been cloning them, or we'd have been seeing similar failures.
So I'll tidy up my hacky implementation, see if it breaks anything elsewhere, and we can go from there.
The first class loader argument (the source class loader) was (iirc) originally used as a test for the class being cloned, but it didn't end up being usable in that capacity. Now it's only used for cases where the actual class loader instance appears, and it's substituted with the target class loader. I don't recall if/why that is needed though; we might be able to just drop that functionality completely and get rid of the argument.
I made a crude attempt at this (just passing through the application class loader for both arguments), and I can confirm the first argument is being used, and has an effect. :)
Darn.
I think this is the right approach. I've got the affected test passing with a variation of this 🎉 🎉 🎉. Just not-replicating
RuntimeUpdatesProcessorwasn't enough, so I went further and didn't replicate any Quarkus classes. I think there's probably a performance gain from not replicating them, and they're already in the 'right' classloader, so I don't think there's a functional reason to replicate them. I'm also fairly confident the previous implementation couldn't have been cloning them, or we'd have been seeing similar failures.
Interesting! The black box becomes ever so slightly less opaque. :-)
So I'll tidy up my hacky implementation, see if it breaks anything elsewhere, and we can go from there.
Awesome, thanks for looking into it.
Like many things, the fix was obvious in hindsight. Sadly, it involves putting back some of the code @dmlloyd removed, but only some. We were seeing problems in the new code because of trying to clone Quarkus classes to other classloaders. With all the static instances and asynchronous initialisations and other skulduggery going on inside Quarkus, those infrastructure classes were never going to appreciate being moved to another classloader.
Another reason not to clone the Quarkus runtime classes themselves is that it’s a performance drag. We want to clone the minimum number of things possible.
The previous, pre-#40601, implementation managed this by doing a whole bunch of checks to decide what to clone. Config could be used to opt things in. More importantly, this check excluded anything already loaded by the main quarkus classloader:
cloneRequired = runningQuarkusApplication.getClassLoader()
.loadClass(theclass.getName()) != theclass
I’ve reinstated those checks exactly as they were before. They do overlap some of the checks in NewSerializingDeepClone, such as the primitive guard and the TestInfo handling, so I’d like to do a follow-up PR which consolidates the two sets of checks (ideally outside of QuarkusTestExtension, which is huge). But in the interests of getting this in promptly, let’s go for the minimal change and leave the original QuarkusTestExtension code in place. I think we’ll need to think a little bit about how the existing ‘opt in to cloning’ config would work in the new ‘clone always except in these situations` world. Maybe we’d just get rid of the config?
@dmlloyd, I squashed the changes on this PR, but you can see them in https://github.com/quarkusio/quarkus/pull/40906/commits/31e9b4f0c48d6f15eec78f51f2b7f1c3fecd3edd.
I don't think the failure in the CI can be anything to do with these changes. All tests on one machine failed with:
Error: io.quarkus.it.main.ServletITCase.null
Error: Run 1: ServletITCase » IllegalState Unable to locate the artifact metadata file created that must be created by Quarkus in order to run integration tests. Make sure this test is run after 'mvn package'.
Indeed that looks unrelated!
You can try running the failing job again just to be on the safe side
Indeed that looks unrelated!
You can try running the failing job again just to be on the safe side
I think that needs higher privileges than I have: https://github.com/quarkusio/quarkus/actions/runs/9724907070/job/26841777161?pr=40906 is the failing job, but I could only re-run it by doing a rebase and re-running everything.
Hmm, same failure. I'll rebase to trigger a new build, in case the problem was a glitch in the process that farms work out to the matrix sub-builds.
Sorry to be the bearer of bad news, but I would be surprised if it was a glitch. Never seen than before and I wouldn't be surprised if it was an actual issue.
The IT main thing is a bit of a weird beast (quite large and mixing quite a lot of extensions/patterns).
Sorry to be the bearer of bad news, but I would be surprised if it was a glitch. Never seen than before and I wouldn't be surprised if it was an actual issue.
What makes me suspect it's a glitch is that it appears to be a packaging issue (the error message claims a file is missing on disk). If a code-level issue in this change affected discoverability of the file, it should affect quite a few sub-builds, not just the one. The failure is across the board in one 'child', but the identical child with virtual threads enabled passes fine, as do all the other native children.
This change clones fewer classes than #40601 (which clones a lot more classes than HEAD), so I can imagine a mechanism where the resource ends up in the wrong classloader, and the integration test are doing getResource() in the other classloader and can't find it. But I've checked the code and the code which looks for the file is looking on disk:
public static Properties readQuarkusArtifactProperties(ExtensionContext context) {
Path buildOutputDirectory = determineBuildOutputDirectory(context);
Path artifactProperties = buildOutputDirectory.resolve("quarkus-artifact.properties");
if (!Files.exists(artifactProperties)) {
The IT main thing is a bit of a weird beast (quite large and mixing quite a lot of extensions/patterns).
I'm trying to reproduce locally, but without luck so far; I've given podman machine 4gb and am running with
MAVEN_OPTS=-Xmx6g mvn -pl integration-tests/main -Dtest-containers -Dstart-containers -Dquarkus.native.native-image-xmx=8g -Dnative -Dnative.surefire.skip -Dno-descriptor-tests -DskipDocs -Dquarkus.native.container-build=true verify
and my local build is OOMing:
[ERROR] Failed to execute goal io.quarkus:quarkus-maven-plugin:999-SNAPSHOT:build (default) on project quarkus-integration-test-main: Failed to build quarkus application: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
[ERROR] [error]: Build step io.quarkus.deployment.pkg.steps.NativeImageBuildStep#build threw an exception: io.quarkus.deployment.pkg.steps.NativeImageBuildStep$ImageGenerationFailureException: Image generation failed. Exit code was 137 which indicates an out of memory error. The most likely cause is Docker not being given enough memory. Also consider increasing the Xmx value for native image generation by setting the "quarkus.native.native-image-xmx" property
[ERROR] at io.quarkus.deployment.pkg.steps.NativeImageBuildStep.imageGenerationFailed(NativeImageBuildStep.java:462)
:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.
Status for workflow Quarkus CI
This is the status report for running Quarkus CI on commit 9ce7c8ea35dceb506d37dec0b1899478ed8453a5.
Failing Jobs
| Status | Name | Step | Failures | Logs | Raw logs | Build scan |
|---|---|---|---|---|---|---|
| ✖ | Initial JDK 17 Build | Build |
Failures | Logs | Raw logs | :mag: |
You can consult the Develocity build scans.
Failures
:gear: Initial JDK 17 Build #
- Failing: test-framework/junit5
! Skipped: devtools/bom-descriptor-json devtools/cli docs and 495 more
:package: test-framework/junit5
✖ Failed to execute goal net.revelc.code:impsort-maven-plugin:1.11.0:check (check-imports) on project quarkus-junit5: Imports are not sorted in /home/runner/work/quarkus/quarkus/test-framework/junit5/src/main/java/io/quarkus/test/junit/QuarkusTestExtension.java
I think we are experiencing this very issue elsewhere: https://github.com/quarkusio/quarkus/pull/41561 .
I think it's related to the Develocity cache extension that is being developed by the Gradle team and @aloubyansky (cc @jprinet). I will have a look.