-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Support JDK serialization/deserialization features #2730
Conversation
Thank you for your contribution @ziyilin ! I've left some first comments and I'm looking forward to having a closer look. |
substratevm/src/com.oracle.svm.agent/src/com/oracle/svm/agent/BreakpointInterceptor.java
Outdated
Show resolved
Hide resolved
...tevm/src/com.oracle.svm.configure/src/com/oracle/svm/configure/config/TypeConfiguration.java
Outdated
Show resolved
Hide resolved
substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/JDK9OrLater.java
Outdated
Show resolved
Hide resolved
...vm/src/com.oracle.svm.reflect/src/com/oracle/svm/reflect/serialize/SerializationSupport.java
Outdated
Show resolved
Hide resolved
The style checker reports several errors, some of which break builds, please see: https://travis-ci.org/github/oracle/graal/jobs/718831254#L2394-L2434 |
substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/FallbackFeature.java
Outdated
Show resolved
Hide resolved
...acle.svm.core/src/com/oracle/svm/core/jdk/Target_jdk_internal_reflect_AccessorGenerator.java
Show resolved
Hide resolved
...m.core/src/com/oracle/svm/core/jdk/serialize/Target_sun_reflect_MethodAccessorGenerator.java
Outdated
Show resolved
Hide resolved
substratevm/src/com.oracle.svm.agent/src/com/oracle/svm/agent/BreakpointInterceptor.java
Outdated
Show resolved
Hide resolved
.../src/com.oracle.svm.agent/src/com/oracle/svm/agent/restrict/SerializationAccessVerifier.java
Outdated
Show resolved
Hide resolved
...tevm/src/com.oracle.svm.configure/src/com/oracle/svm/configure/config/TypeConfiguration.java
Outdated
Show resolved
Hide resolved
fixed |
5df9526
to
b2c20b2
Compare
.../src/com.oracle.svm.agent/src/com/oracle/svm/agent/restrict/SerializationAccessVerifier.java
Outdated
Show resolved
Hide resolved
...com.oracle.svm.configure/src/com/oracle/svm/configure/config/SerializationConfiguration.java
Outdated
Show resolved
Hide resolved
substratevm/src/com.oracle.svm.configure/src/com/oracle/svm/configure/trace/AccessAdvisor.java
Outdated
Show resolved
Hide resolved
...evm/src/com.oracle.svm.configure/src/com/oracle/svm/configure/trace/ReflectionProcessor.java
Outdated
Show resolved
Hide resolved
substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/DynamicHub.java
Outdated
Show resolved
Hide resolved
b83c4e0
to
13c898b
Compare
...com.oracle.svm.reflect/src/com/oracle/svm/reflect/serialize/hosted/SerializationFeature.java
Outdated
Show resolved
Hide resolved
@ziyilin thanks for investigating further. There is now a PR on master that allows us to defer illegal NewInstance to a image runtime error with To have serialization in |
9948ed8
to
e42caa9
Compare
...com.oracle.svm.configure/src/com/oracle/svm/configure/config/SerializationConfiguration.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good. One small thing left. See #2730 (comment)
@olpaw I have fixed the "new abstract class" issue. |
...vm/src/com.oracle.svm.reflect/src/com/oracle/svm/reflect/serialize/SerializationSupport.java
Show resolved
Hide resolved
That matches with my observation. Using 3f76cff with Glad you found a proper fix so we do not need to rely on |
e42caa9
to
c4fb66c
Compare
@ziyilin created internal PR. Thanks a lot for your contribution. |
@peter-hofer @olpaw I have a compatibility concern about the serialization feature. Native image is based on labs-openJDK or graal-jvmci-8 which are not exactly the same as OpenJDK. Is there any chance a serialized target class is a JDK class that has different contents between OpenJDK and labs-openJDK? In this case, the serialization data from native image could be different from openJDK, leading to inconsistent runtime behaviors. |
@ziyilin It's possible, but unlikely. The Labs JDKs generally incorporate JVMCI changes and various bugfixes and not changes to JDK classes, especially not those classes which we would expect to be used in serialization, like collections. These are typically also written with serialization in mind (transient fields, |
@peter-hofer Thanks for explaining. In this case, I don't need to worry about the compatible problem. |
JDK's serialization/deserialization features are implemented by
java.io.java.io.ObjectInputStream.readObject
andjava.io.ObjectOutputStream.writeObject
APIs which are not supported by native image. This patch supports these two APIs in native image.Features:
Ljava/io/ObjectOutputStream;#writeObject(Ljava/lang/Object;)V
Ljava/io/ObjectInputStream;#readObject()Ljava/lang/Object;
serialization-config.json
to provide serialization/deserialization target class information forcom.alibaba.test.serialze.Data
", but one extendscom.alibaba.test.serialize.DummyBase
and another doesn't. When they are loaded and serialized by different classloaders. the following error will be reported at build time:Tests:
Tests are here:
tests.zip
There are 4 tests in tests.zip for this patch. Unzip the file and run each shell script started with "test" to see the result.