fix(java): support copy of non-serializable JDK classes such as java.lang.Package#3797
Open
sjahedhussini wants to merge 7 commits into
Open
fix(java): support copy of non-serializable JDK classes such as java.lang.Package#3797sjahedhussini wants to merge 7 commits into
sjahedhussini wants to merge 7 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why?
Fory.copy(obj)throwsUnsupportedOperationException: Class java.lang.Package doesn't support serialization(wrapped inCopyException) when the object graphcontains a non-
SerializableJDK class, even though the same object round-tripsfine through
serialize+deserialize. This makes deep-copying commonreal-world object graphs (e.g. Hibernate objects) fail.
What does this PR do?
Root cause:
ClassResolver.getSerializerClassrejects non-SerializableJDKclasses via the
checkJdkClassSerializableguard. The check is correct forserialization, but because copy and serialize share the same per-class serializer
created in
createSerializer, the guard fired at serializer-creation time andbroke
copy()as collateral damage.Fix: route these classes to a new
NonSerializableSerializer. Itswrite/readstill throw
UnsupportedOperationException(binary serialization behavior isunchanged — the failure is simply deferred to write time), but it inherits the
field-copy implementation from
AbstractObjectSerializer, soFory.copy()nowworks. Transient fields are still copied (e.g.
HashMap'ssize/table), sincethe inherited copy path copies all non-static fields.
NonSerializableSerializeris kept deliberately distinct fromCopyOnlyObjectSerializer: the latter blocks serialization forregistration-security reasons (remediable by registering the class), whereas a
non-
SerializableJDK class is intrinsically non-serializable and not remediableby registration, so the failure semantics differ. The new serializer is also
registered in
GraalvmSupport's default serializer set, consistent with everyother class returned by
getSerializerClass.Files changed:
resolver/ClassResolver.java— route non-SerializableJDK classes to the new serializer instead of throwing.serializer/NonSerializableSerializer.java— new serializer (copy supported, serialize unsupported).platform/GraalvmSupport.java— register the new serializer for native-image builds.test/ForyCopyTest.java— tests.Behavior note: serialization of these classes now surfaces as
SerializationExceptionwrappingUnsupportedOperationException, rather than theraw
UnsupportedOperationExceptionthrown eagerly at serializer creation.Related issues
Fixes issue #2941.
AI Contribution Checklist
Substantial AI assistance was used in this PR:
yesIf
yes, I included a completed AI Contribution Checklist in this PR description and the requiredAI Usage Disclosure.If
yes, my PR description includes theai_reviewsummary and screenshot evidence/links from both fresh reviewers on the current diff.Substantial AI assistance was used:
yesI can explain and defend all important changes without AI help.
I reviewed AI-assisted code changes line by line before submission.
I completed line-by-line self-review first and fixed issues before requesting AI review.
I ran two fresh AI review agents on the current diff: one using
.claude/skills/fory-code-review/SKILL.mdand one without.I addressed all AI review comments and repeated the loop until both reviewers reported no further actionable comments.
I attached evidence of the final clean AI review from both reviewers below.
I ran human verification and recorded evidence below.
I added/updated tests where required.
N/A — no protocol/wire-format change; no separate performance evidence required (see below).
I verified licensing and provenance compliance.
```
AI Usage Disclosure
Do a thorough, independent code review of the changes on my current branch
versus main:
git diff main...fix-issue-2941. Review every changed line forcorrectness, edge cases, performance, maintainability, test quality, and style.
Do NOT use any project-specific review skill — I want a fresh general review.
List concrete actionable issues.
The claude command for SKILL based agent is:
Review the changes on my current branch versus main using the guidance in
.claude/skills/fory-code-review/SKILL.md. Diff:
git diff main...fix-issue-2941.Go line by line and list any actionable issues.
```
Does this PR introduce any user-facing change?
Fory.copy()now succeeds where it previously threw; no signature or contract change.Benchmark
N/A — see performance_verification above.