Fix crash

apache · Oct 4, 2023 · c89b841 · c89b841
1 parent 13e90cc
commit c89b841
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 18 deletions.
diff --git a/java/fury-core/src/main/java/io/fury/memory/MemoryBuffer.java b/java/fury-core/src/main/java/io/fury/memory/MemoryBuffer.java
@@ -1193,23 +1193,27 @@ public void writeDouble(double value) {
    * #writePositiveVarInt} to save one bit.
    */
   public int writeVarInt(int v) {
-    ensure(writerIndex + 5);
+    ensure(writerIndex + 8);
     return unsafeWriteVarInt(v);
   }
 
   /**
-   * Writes a 1-9 byte int.
+   * Writes a 1-5 byte int.
    *
    * @return The number of bytes written.
    */
   public int writePositiveVarInt(int v) {
-    // ensure at least 9 bytes are writable at once, so jvm-jit
+    // ensure at least 8 bytes are writable at once, so jvm-jit
     // generated code is smaller. Otherwise, `MapRefResolver.writeRefOrNull`
     // may be `callee is too large`/`already compiled into a big method`
-    ensure(writerIndex + 5);
+    ensure(writerIndex + 8);
     return unsafeWritePositiveVarInt(v);
   }
 
+  /**
+   * For implementation efficiency, this method needs at most 8 bytes for writing 5 bytes
+   * using long to avoid using two memory operations.
+   */
   public int unsafeWriteVarInt(int v) {
     // Ensure negatives close to zero is encode in little bytes.
     v = (v << 1) ^ (v >> 31);
@@ -1222,6 +1226,10 @@ public int readVarInt() {
     return (r >>> 1) ^ -(r & 1);
   }
 
+  /**
+   * For implementation efficiency, this method needs at most 8 bytes for writing 5 bytes
+   * using long to avoid using two memory operations.
+   */
   public int unsafeWritePositiveVarInt(int v) {
     // The encoding algorithm are based on kryo UnsafeMemoryOutput.writeVarInt
     // varint are written using little endian byte order.

diff --git a/java/fury-core/src/test/java/io/fury/FuryTest.java b/java/fury-core/src/test/java/io/fury/FuryTest.java
@@ -464,9 +464,18 @@ private void furyGC(WeakHashMap<Object, Boolean> map) {
 
   @Test
   public void testSerializeJavaObject() {
-    Fury fury = Fury.builder().requireClassRegistration(false).withLanguage(Language.JAVA).build();
+    Fury fury =
+        Fury.builder()
+            .requireClassRegistration(false)
+            .withClassVersionCheck(false)
+            .withLanguage(Language.JAVA)
+            .build();
     BeanA beanA = BeanA.createBeanA(2);
     assertEquals(fury.deserializeJavaObject(fury.serializeJavaObject(beanA), BeanA.class), beanA);
+    // Without class version check, janino generated code may crash jvm.
+    // The ArraySerializers.IntArraySerializer will read num elements: 153756481.
+    // For some reason, if BeanACodec are compiled iby janino in JDK11, it will crash.
+    // but if compile BeanACodec using javac, no crash will happen.
     assertThrows(
         Exception.class,
         () -> fury.deserializeJavaObject(fury.serializeJavaObjectAndClass(beanA), BeanA.class));

diff --git a/java/fury-core/src/test/java/io/fury/serializer/UnexistedClassSerializersTest.java b/java/fury-core/src/test/java/io/fury/serializer/UnexistedClassSerializersTest.java
@@ -106,12 +106,8 @@ public void testDeserializeUnexistedNewFury(
     ClassLoader classLoader = getClass().getClassLoader();
     for (Class<?> structClass :
         new Class<?>[] {
-          // Serialization may crash at `G1ParScanThreadState::copy_to_survivor_space` in
-          // ubuntu22 and jdk11/17. It's a jvm bug, see:
-          // https://github.com/alipay/fury/pull/923#issuecomment-1745035339
-          // Workaround by disable cache.
-          Struct.createNumberStructClass("TestSkipUnexistedClass2", 2, false),
-          Struct.createStructClass("TestSkipUnexistedClass2", 2, false)
+          Struct.createNumberStructClass("TestSkipUnexistedClass2", 2),
+          Struct.createStructClass("TestSkipUnexistedClass2", 2)
         }) {
       Object pojo = Struct.createPOJO(structClass);
       MetaContext context1 = new MetaContext();
@@ -163,12 +159,8 @@ public void testDeserializeUnexisted(
     ClassLoader classLoader = getClass().getClassLoader();
     for (Class<?> structClass :
         new Class<?>[] {
-          // Serialization may crash at `G1ParScanThreadState::copy_to_survivor_space` in
-          // ubuntu22 and jdk11/17. It's a jvm bug, see:
-          // https://github.com/alipay/fury/pull/923#issuecomment-1745035339
-          // Workaround by disable cache.
-          Struct.createNumberStructClass("TestSkipUnexistedClass3", 2, false),
-          Struct.createStructClass("TestSkipUnexistedClass3", 2, false)
+          Struct.createNumberStructClass("TestSkipUnexistedClass3", 2),
+          Struct.createStructClass("TestSkipUnexistedClass3", 2)
         }) {
       Fury fury2 =
           builder()
@@ -207,7 +199,7 @@ public void testDeserializeUnexisted(
   public void testThrowExceptionIfClassNotExist() {
     Fury fury = builder().withDeserializeUnexistedClass(false).build();
     ClassLoader classLoader = getClass().getClassLoader();
-    Class<?> structClass = Struct.createNumberStructClass("TestSkipUnexistedClass1", 2, false);
+    Class<?> structClass = Struct.createNumberStructClass("TestSkipUnexistedClass1", 2);
     Object pojo = Struct.createPOJO(structClass);
     Fury fury2 =
         builder().withDeserializeUnexistedClass(false).withClassLoader(classLoader).build();