Skip to content

Conversation

@kitalkuyo-gita
Copy link

Why?

What does this PR do?

Related issues

Related to issue-2622

Does this PR introduce any user-facing change?

  • Does this PR introduce any public API change?
  • Does this PR introduce any binary protocol compatibility change?

Benchmark

@kitalkuyo-gita
Copy link
Author

kitalkuyo-gita commented Oct 22, 2025

Great! Could we remove https://github.com/apache/fory/blob/main/java/fory-core/src/main/java/org/apache/fory/serializer/CompatibleSerializer.java and https://github.com/apache/fory/blob/main/java/fory-core/src/main/java/org/apache/fory/resolver/FieldResolver.java in this PR too?

Hello, @chaokunyang . I see these two classes are marked as deprecated. However, I believe further discussion is needed regarding their removal. Completely removing these classes would impact backward compatibility. Therefore, I recommend retaining the CompatibleSerializer in non-meta-shared mode and using the ObjectStreamMetaSharedSerializerAdapter only in meta-shared mode.

Restore the conditional for useMetaShare in SlotsInfo.

Non-meta-shared mode: Use the CompatibleSerializer.

Meta-shared mode: Use the ObjectStreamMetaSharedSerializerAdapter.

What do you think about this?

@kitalkuyo-gita
Copy link
Author

While fixing CI, I encountered a problem. LinkedBlockingQueue and ArrayBlockingQueue both have custom writeObject/readObject methods. According to requireJavaSerialization , they use ObjectStreamSerializer . However, the LinkedBlockingQueue.readObject method calls the add() method during deserialization, which requires a lock. If the serialization lock state is incorrect, a deadlock will occur. The key issue is that LinkedBlockingQueue and ArrayBlockingQueue are not suitable for using ObjectStreamSerializer . JavaSerializer (full JDK serialization) should be used instead.

@kitalkuyo-gita
Copy link
Author

@chaokunyang Hello, it's ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants