chaokunyang commented on code in PR #2852:
URL: https://github.com/apache/fory/pull/2852#discussion_r2476966688
##########
java/fory-core/src/test/java/org/apache/fory/ThreadSafeForyTest.java:
##########
@@ -373,4 +373,137 @@ public void testSerializerRegister() {
return null;
});
}
+
+ @Test
+ public void testRegisterClassAfterInitialization() throws
InterruptedException {
+ ExecutorService executor = Executors.newSingleThreadExecutor();
+ AtomicReference<Throwable> ex = new AtomicReference<>();
+
+ try {
+ ThreadSafeFory fory =
Fory.builder().requireClassRegistration(true).buildThreadLocalFory();
+
+ fory.register(BeanB.class);
+
+ BeanB bean = BeanB.createBeanB(2);
+ byte[] bytes = fory.serialize(bean);
+
+ executor.execute(
+ () -> {
+ try {
+ Object deserialized = fory.deserialize(bytes);
+ Assert.assertEquals(deserialized, bean);
+ } catch (Throwable t) {
+ ex.set(t);
+ }
+ });
+
+ executor.shutdown();
+ assertTrue(executor.awaitTermination(10, TimeUnit.SECONDS));
+
+ if (ex.get() != null) {
+ throw new AssertionError("error", ex.get());
+ }
+ } finally {
+ if (!executor.isTerminated()) {
+ executor.shutdownNow();
+ }
+ }
+ }
+
+ @Test
+ public void testConcurrentRegisterAndDeserialize() throws
InterruptedException {
+ ExecutorService executor = Executors.newFixedThreadPool(4);
+ ConcurrentHashMap<String, Throwable> errors = new ConcurrentHashMap<>();
+
+ try {
+ ThreadSafeFory fory =
Fory.builder().requireClassRegistration(true).buildThreadLocalFory();
+
+ fory.register(BeanA.class);
+ fory.register(BeanB.class);
+ BeanA beanA = BeanA.createBeanA(2);
+ byte[] bytesA = fory.serialize(beanA);
Review Comment:
You only register once, then serialize with that Fory instance multiple
times. This should work as before. I don't think this PR fix anything.
IMO, the real issue is that the `serialize` has been executed a few times,
but after that, users call `register` method. Such pattern are not allowed, no
register should be invoked after any serialize/deserialize has been executed.
As for register itself, may just add `synchronized` to method is enough
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
