Allow reflection for native serialization #19942

vsevel · 2021-09-06T19:40:02Z

Fixes #19711

I have assumed that for reflection we needed constructor=true and we honor annotation parameters methods and fields (just like we do for the non serialization case).
I kept finalFieldsWritable and weak false. weak and serialization appear to be exclusive.
I am not sure finalFieldsWritable makes sense with serialization.

/cc @jaikiran @JiriOndrusek @stuartwdouglas

quarkus-bot · 2021-09-06T23:28:37Z

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building `8e4b484`

Status	Name	Step	Failures	Logs	Raw logs
✖	JVM Tests - JDK 11	`Build`	Failures	Logs	Raw logs
✖	JVM Tests - JDK 16	`Build`	Failures	Logs	Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 11 #

- Failing: extensions/vertx-http/deployment 
! Skipped: core/test-extension/deployment docs extensions/agroal/deployment and 287 more

📦 extensions/vertx-http/deployment

✖ io.quarkus.vertx.http.testrunner.tags.ExcludeTagsTestCase.checkTestsAreRun line 65 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Failed to wait for test run 3 State{lastRun=2, running=true, inProgress=false, run=5, passed=5, failed=0, skipped=0, isBrokenOnly=false, isTestOutput=false, isInstrumentationBasedReload=false, isLiveReload=true}
	at io.quarkus.test.ContinuousTestingTestUtils.waitForNextCompletion(ContinuousTestingTestUtils.java:43)
	at io.quarkus.vertx.http.testrunner.tags.ExcludeTagsTestCase.checkTestsAreRun(ExcludeTagsTestCase.java:65)

⚙️ JVM Tests - JDK 16 #

- Failing: extensions/vertx-http/deployment 
! Skipped: core/test-extension/deployment docs extensions/agroal/deployment and 287 more

📦 extensions/vertx-http/deployment

✖ io.quarkus.vertx.http.testrunner.tags.ExcludeTagsTestCase.checkTestsAreRun line 54 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Failed to wait for test run 2 State{lastRun=1, running=true, inProgress=false, run=3, passed=3, failed=0, skipped=0, isBrokenOnly=false, isTestOutput=false, isInstrumentationBasedReload=false, isLiveReload=true}
	at io.quarkus.test.ContinuousTestingTestUtils.waitForNextCompletion(ContinuousTestingTestUtils.java:43)
	at io.quarkus.vertx.http.testrunner.tags.ExcludeTagsTestCase.checkTestsAreRun(ExcludeTagsTestCase.java:54)

quarkus-bot · 2021-09-07T09:56:10Z

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building `8e4b484`

Status	Name	Step	Failures	Logs	Raw logs
✖	JVM Tests - JDK 11	`Build`	Failures	Logs	Raw logs
✔️	JVM Tests - JDK 16

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 11 #

- Failing: extensions/smallrye-reactive-messaging-kafka/deployment 
! Skipped: docs integration-tests/kubernetes/quarkus-standard-way-kafka integration-tests/reactive-messaging-kafka and 1 more

📦 extensions/smallrye-reactive-messaging-kafka/deployment

✖ io.quarkus.smallrye.reactivemessaging.kafka.deployment.testing.KafkaDevServicesContinuousTestingTestCase.testContinuousTestingScenario2 line 75 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Failed to wait for test run 2 State{lastRun=1, running=true, inProgress=false, run=1, passed=0, failed=1, skipped=0, isBrokenOnly=false, isTestOutput=false, isInstrumentationBasedReload=false, isLiveReload=true}
	at io.quarkus.test.ContinuousTestingTestUtils.waitForNextCompletion(ContinuousTestingTestUtils.java:43)
	at io.quarkus.smallrye.reactivemessaging.kafka.deployment.testing.KafkaDevServicesContinuousTestingTestCase.testContinuousTestingScenario2(KafkaDevServicesContinuousTestingTestCase.java:75)

quarkus-bot · 2021-09-07T17:01:45Z

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building `34161f0`

Status	Name	Step	Failures	Logs	Raw logs
✔️	JVM Tests - JDK 11
✖	JVM Tests - JDK 11 Windows	`Build`	Failures	Logs	Raw logs
✔️	JVM Tests - JDK 16

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 11 Windows #

- Failing: extensions/amazon-lambda/deployment 
! Skipped: docs extensions/amazon-lambda-http/deployment extensions/amazon-lambda-rest/deployment and 6 more

📦 extensions/amazon-lambda/deployment

✖ io.quarkus.amazon.lambda.deployment.testing.LambdaDevServicesContinuousTestingTestCase.testLambda line 41 - More details - Source on GitHub

org.opentest4j.AssertionFailedError: expected: <0> but was: <1>
	at org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)
	at org.junit.jupiter.api.AssertionUtils.failNotEqual(AssertionUtils.java:62)

jaikiran · 2021-09-08T02:20:40Z

Hello @vsevel, this looks OK to me at first glance. However, I haven't looked at the deserialization code in JDK and what other reflection access other than constructors might be needed. I think what would be good and definitive is a simple test case (in native mode) which reproduces the original issue and then verifies this change to make sure the serialization (and internally reflection) works as expected. We already have native tests which run REST endpoints, so I guess those existing ones can be enhanced to test this usecase, maybe?

vsevel · 2021-09-08T11:23:45Z

hello @jaikiran I have added a list field in the SomeSerializationObject class used in the CoreSerializationInGraalITCase. unfortunately the test fails. I searched quite a bit but could not figure out what is going on. I have added some logging in the ReflectiveClassBuildItem, NativeImageAutoFeatureStep and RegisterForReflectionBuildStep to make sure the annotation was properly processed, and produced the appropriate build items.

@RegisterForReflection(targets = { List.class, ArrayList.class, String.class }, serialization = true)
public class SerializationConfig {

I decompiled also the fat jar, and checked that the AutoFeature had the following:

    private static void registerClass1(BeforeAnalysisAccess var0) {
        try {
            ClassLoader var1 = Thread.currentThread().getContextClassLoader();
            Class var2 = Class.forName("java.util.ArrayList", (boolean)0, var1);
            Constructor[] var4 = var2.getDeclaredConstructors();
            Method[] var5 = var2.getDeclaredMethods();
            Field[] var8 = var2.getDeclaredFields();
            Class[] var3 = new Class[]{var2};
            RuntimeReflection.register(var3);
            RuntimeReflection.register((Executable[])var4);
            RuntimeReflection.register((Executable[])var5);
            Version var6 = Version.getCurrent();
            int[] var7 = new int[]{21};
            if (var6.compareTo(var7) < 0) {
                RuntimeReflection.register((boolean)0, var8);
            } else {
                RuntimeReflection.register((boolean)0, (boolean)1, var8);
            }

            registerSerializationForClass(var2);
        } catch (Throwable var9) {
        }

    }

Note that constructors get registered for the ArrayList.

Yet, the test keeps failing with:

[ERROR]   CoreSerializationInGraalITCase.testEntitySerializationFromServlet:16 1 expectation failed.
Response body doesn't match expectation.
Expected: is "OK"
  Actual: java.io.InvalidClassException: java.util.ArrayList; no valid constructor
        java.io.InvalidClassException: java.util.ArrayList; no valid constructor
        at java.io.ObjectStreamClass$ExceptionInfo.newInvalidClassException(ObjectStreamClass.java:159)
        at java.io.ObjectStreamClass.checkDeserialize(ObjectStreamClass.java:875)
        at java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:2170)
        at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1679)
        at java.io.ObjectInputStream.defaultReadFields(ObjectInputStream.java:2464)
        at java.io.ObjectInputStream.readSerialData(ObjectInputStream.java:2358)
        at java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:2196)
        at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1679)
        at java.io.ObjectInputStream.readObject(ObjectInputStream.java:493)
        at java.io.ObjectInputStream.readObject(ObjectInputStream.java:451)
        at io.quarkus.it.corestuff.SerializationTestEndpoint.reflectiveSetterInvoke(SerializationTestEndpoint.java:53)

Could this be an issue in graalvm itself?

quarkus-bot · 2021-09-08T14:41:54Z

Failing Jobs - Building `e620fdb`

Status	Name	Step	Failures	Logs	Raw logs
✖	MicroProfile TCKs Tests	`Verify`	Failures	Logs	Raw logs
✖	Native Tests - Main	`Build`	Failures	Logs	Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ MicroProfile TCKs Tests #

- Failing: tcks/microprofile-config

📦 tcks/microprofile-config

✖ org.eclipse.microprofile.config.tck.ConfigPropertiesTest.testConfigPropertiesDefaultOnBean line 172 - More details - Source on GitHub

java.lang.NullPointerException
	at org.eclipse.microprofile.config.tck.ConfigPropertiesTest.testConfigPropertiesDefaultOnBean(ConfigPropertiesTest.java:172)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)

✖ org.eclipse.microprofile.config.tck.ConfigPropertiesTest.testConfigPropertiesNoPrefixOnBean line 149 - More details - Source on GitHub

java.lang.NullPointerException
	at org.eclipse.microprofile.config.tck.ConfigPropertiesTest.testConfigPropertiesNoPrefixOnBean(ConfigPropertiesTest.java:149)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)

✖ org.eclipse.microprofile.config.tck.ConfigPropertiesTest.testConfigPropertiesNoPrefixOnBeanThenSupplyPrefix line 156 - More details - Source on GitHub

java.lang.NullPointerException
	at org.eclipse.microprofile.config.tck.ConfigPropertiesTest.testConfigPropertiesNoPrefixOnBeanThenSupplyPrefix(ConfigPropertiesTest.java:156)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)

✖ org.eclipse.microprofile.config.tck.ConfigPropertiesTest.testConfigPropertiesPlainInjection line 106 - More details - Source on GitHub

java.lang.NullPointerException
	at org.eclipse.microprofile.config.tck.ConfigPropertiesTest.testConfigPropertiesPlainInjection(ConfigPropertiesTest.java:106)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)

✖ org.eclipse.microprofile.config.tck.ConfigPropertiesTest.testConfigPropertiesWithPrefix line 115 - More details - Source on GitHub

java.lang.NullPointerException
	at org.eclipse.microprofile.config.tck.ConfigPropertiesTest.testConfigPropertiesWithPrefix(ConfigPropertiesTest.java:115)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)

✖ org.eclipse.microprofile.config.tck.ConfigPropertiesTest.testConfigPropertiesWithoutPrefix line 132 - More details - Source on GitHub

java.lang.NullPointerException
	at org.eclipse.microprofile.config.tck.ConfigPropertiesTest.testConfigPropertiesWithoutPrefix(ConfigPropertiesTest.java:132)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)

✖ org.eclipse.microprofile.config.tck.ConfigPropertiesTest.testNoConfigPropertiesAnnotationInjection line 165 - More details - Source on GitHub

java.lang.NullPointerException
	at org.eclipse.microprofile.config.tck.ConfigPropertiesTest.testNoConfigPropertiesAnnotationInjection(ConfigPropertiesTest.java:165)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)

⚙️ Native Tests - Main #

- Failing: integration-tests/main

📦 integration-tests/main

✖ io.quarkus.it.main.CoreSerializationInGraalITCase.testEntitySerializationFromServlet line 16 - More details - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Response body doesn't match expectation.

zakkak · 2021-09-15T10:17:22Z

Could this be an issue in graalvm itself?

No, that's not an issue in GraalVM (it's also not reproducible with 21.3-dev where we use RuntimeSerialization.register to register classes for serialization support). The issue appears to be Quarkus passing the wrong argument to com.oracle.svm.reflect.serialize.hosted.SerializationFeature.addReflections in

quarkus/core/deployment/src/main/java/io/quarkus/deployment/steps/NativeImageAutoFeatureStep.java

Lines 672 to 674 in 550e59a

    
           tc.writeArrayValue(addReflectionsArgs2, 0, clazz); 
        
           tc.writeArrayValue(addReflectionsArgs2, 1, objectClass); 
        
           tc.invokeVirtualMethod(invokeMethodDescriptor, addReflectionsLookupMethod, tc.loadNull(), addReflectionsArgs2);

I created #20167 to fix it.

zakkak · 2021-09-21T11:47:35Z

Closing, since #20167, which fixes the immediate issue, is now merged.
I have also created #20300 to request a new annotation for registering classes for serialization.

quarkus-bot bot added the area/core label Sep 6, 2021

Allow reflection for native serialization

34161f0

test

e620fdb

This was referenced Sep 14, 2021

GraalVM issue with ArrayList serialization #19711

Closed

Fix reflections registration of constructors used in serialization #20167

Merged

zakkak closed this Sep 21, 2021

quarkus-bot bot added the triage/invalid This doesn't seem right label Sep 21, 2021

vsevel deleted the serialization branch September 21, 2021 16:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow reflection for native serialization #19942

Allow reflection for native serialization #19942

vsevel commented Sep 6, 2021 •

edited

Loading

quarkus-bot bot commented Sep 6, 2021 •

edited

Loading

Failing Jobs - Building `8e4b484`

Failures

⚙️ JVM Tests - JDK 11 #

📦 extensions/vertx-http/deployment

⚙️ JVM Tests - JDK 16 #

📦 extensions/vertx-http/deployment

quarkus-bot bot commented Sep 7, 2021 •

edited

Loading

Failing Jobs - Building `8e4b484`

Failures

⚙️ JVM Tests - JDK 11 #

📦 extensions/smallrye-reactive-messaging-kafka/deployment

quarkus-bot bot commented Sep 7, 2021 •

edited

Loading

Failing Jobs - Building `34161f0`

Failures

⚙️ JVM Tests - JDK 11 Windows #

📦 extensions/amazon-lambda/deployment

jaikiran commented Sep 8, 2021

vsevel commented Sep 8, 2021

quarkus-bot bot commented Sep 8, 2021

zakkak commented Sep 15, 2021 •

edited

Loading

zakkak commented Sep 21, 2021

Allow reflection for native serialization #19942

Allow reflection for native serialization #19942

Conversation

vsevel commented Sep 6, 2021 • edited Loading

quarkus-bot bot commented Sep 6, 2021 • edited Loading

Failing Jobs - Building 8e4b484

Failures

⚙️ JVM Tests - JDK 11 #

📦 extensions/vertx-http/deployment

⚙️ JVM Tests - JDK 16 #

📦 extensions/vertx-http/deployment

quarkus-bot bot commented Sep 7, 2021 • edited Loading

Failing Jobs - Building 8e4b484

Failures

⚙️ JVM Tests - JDK 11 #

📦 extensions/smallrye-reactive-messaging-kafka/deployment

quarkus-bot bot commented Sep 7, 2021 • edited Loading

Failing Jobs - Building 34161f0

Failures

⚙️ JVM Tests - JDK 11 Windows #

📦 extensions/amazon-lambda/deployment

jaikiran commented Sep 8, 2021

vsevel commented Sep 8, 2021

quarkus-bot bot commented Sep 8, 2021

Failing Jobs - Building e620fdb

Failures

⚙️ MicroProfile TCKs Tests #

📦 tcks/microprofile-config

⚙️ Native Tests - Main #

📦 integration-tests/main

zakkak commented Sep 15, 2021 • edited Loading

zakkak commented Sep 21, 2021

vsevel commented Sep 6, 2021 •

edited

Loading

quarkus-bot bot commented Sep 6, 2021 •

edited

Loading

Failing Jobs - Building `8e4b484`

quarkus-bot bot commented Sep 7, 2021 •

edited

Loading

Failing Jobs - Building `8e4b484`

quarkus-bot bot commented Sep 7, 2021 •

edited

Loading

Failing Jobs - Building `34161f0`

Failing Jobs - Building `e620fdb`

zakkak commented Sep 15, 2021 •

edited

Loading