Merge branch 'fixcorruptionbug' of https://github.com/2018ik/lmdbjava into 2018ik-fixcorruptionbug

benalexau · benalexau · commit c5e02f7b0bd5 · 2023-04-24T13:30:40.000+10:00
diff --git a/pom.xml b/pom.xml
@@ -70,6 +70,12 @@
       <groupId>org.hamcrest</groupId>
       <artifactId>hamcrest</artifactId>
     </dependency>
+    <dependency>
+      <groupId>org.mockito</groupId>
+      <artifactId>mockito-inline</artifactId>
+      <version>4.11.0</version>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
   <build>
     <plugins>
@@ -87,6 +93,8 @@
         <configuration>
           <ignoredDependencies>
             <ignoredDependency>com.github.jnr:jffi</ignoredDependency>
+            <ignoredDependency>org.mockito:mockito-core</ignoredDependency>
+            <ignoredDependency>org.mockito:mockito-inline</ignoredDependency>
           </ignoredDependencies>
         </configuration>
       </plugin>
diff --git a/src/main/java/org/lmdbjava/Cursor.java b/src/main/java/org/lmdbjava/Cursor.java
@@ -162,6 +162,7 @@ public boolean get(final T key, final T data, final SeekOp op) {
     checkRc(rc);
     kv.keyOut();
     kv.valOut();
+    ReferenceUtil.reachabilityFence0(key);
     return true;
   }
 
@@ -192,6 +193,7 @@ public boolean get(final T key, final GetOp op) {
     checkRc(rc);
     kv.keyOut();
     kv.valOut();
+    ReferenceUtil.reachabilityFence0(key);
     return true;
   }
 
@@ -266,6 +268,8 @@ public boolean put(final T key, final T val, final PutFlags... op) {
       return false;
     }
     checkRc(rc);
+    ReferenceUtil.reachabilityFence0(key);
+    ReferenceUtil.reachabilityFence0(val);
     return true;
   }
 
@@ -304,6 +308,8 @@ public void putMultiple(final T key, final T val, final int elements,
     final int rc = LIB.mdb_cursor_put(ptrCursor, txn.kv().pointerKey(),
                                       dataPtr, mask);
     checkRc(rc);
+    ReferenceUtil.reachabilityFence0(key);
+    ReferenceUtil.reachabilityFence0(val);
   }
 
   /**
@@ -362,6 +368,7 @@ public T reserve(final T key, final int size, final PutFlags... op) {
     checkRc(LIB.mdb_cursor_put(ptrCursor, kv.pointerKey(), kv.pointerVal(),
                                flags));
     kv.valOut();
+    ReferenceUtil.reachabilityFence0(key);
     return val();
   }
 
diff --git a/src/main/java/org/lmdbjava/Dbi.java b/src/main/java/org/lmdbjava/Dbi.java
@@ -171,6 +171,8 @@ public boolean delete(final Txn<T> txn, final T key, final T val) {
       return false;
     }
     checkRc(rc);
+    ReferenceUtil.reachabilityFence0(key);
+    ReferenceUtil.reachabilityFence0(val);
     return true;
   }
 
@@ -239,6 +241,7 @@ public T get(final Txn<T> txn, final T key) {
       return null;
     }
     checkRc(rc);
+    ReferenceUtil.reachabilityFence0(key);
     return txn.kv().valOut(); // marked as out in LMDB C docs
   }
 
@@ -386,6 +389,8 @@ public boolean put(final Txn<T> txn, final T key, final T val,
       return false;
     }
     checkRc(rc);
+    ReferenceUtil.reachabilityFence0(key);
+    ReferenceUtil.reachabilityFence0(val);
     return true;
   }
 
@@ -421,6 +426,7 @@ public T reserve(final Txn<T> txn, final T key, final int size,
     checkRc(LIB.mdb_put(txn.pointer(), ptr, txn.kv().pointerKey(), txn.kv()
                         .pointerVal(), flags));
     txn.kv().valOut(); // marked as in,out in LMDB C docs
+    ReferenceUtil.reachabilityFence0(key);
     return txn.val();
   }
 
diff --git a/src/main/java/org/lmdbjava/ReferenceUtil.java b/src/main/java/org/lmdbjava/ReferenceUtil.java
@@ -0,0 +1,55 @@
+/*-
+ * #%L
+ * LmdbJava
+ * %%
+ * Copyright (C) 2016 - 2023 The LmdbJava Open Source Project
+ * %%
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * 
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * #L%
+ */
+
+package org.lmdbjava;
+
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+
+public final class ReferenceUtil {
+    /**
+     * Ensures that the object referenced by the given reference remains
+     * <a href="package-summary.html#reachability"><em>strongly reachable</em></a>,
+     * regardless of any prior actions of the program that might otherwise cause
+     * the object to become unreachable; thus, the referenced object is not
+     * reclaimable by garbage collection at least until after the invocation of
+     * this method.
+     *
+     * <p> Recent versions of the JDK have a nasty habit of prematurely deciding objects are unreachable.
+     * see: https://stackoverflow.com/questions/26642153/finalize-called-on-strongly-reachable-object-in-java-8
+     * The Java 9 method Reference.reachabilityFence offers a solution to this problem.
+     *
+     * <p> This method is always implemented as a synchronization on {@code ref}, not as
+     * {@code Reference.reachabilityFence} for consistency across platforms and to allow building on JDK 6-8.
+     * <b>It is the caller's responsibility to ensure that this synchronization will not cause deadlock.</b>
+     *
+     * @param ref the reference. If {@code null}, this method has no effect.
+     * @see https://github.com/netty/netty/pull/8410
+     */
+    @SuppressFBWarnings({"ESync_EMPTY_SYNC", "UC_USELESS_VOID_METHOD"})
+    public static void reachabilityFence0(Object ref) {
+        if (ref != null) {
+            synchronized (ref) {
+                // Empty synchronized is ok: https://stackoverflow.com/a/31933260/1151521
+            }
+        }
+    }
+
+    private ReferenceUtil() {}
+}
diff --git a/src/test/java/org/lmdbjava/GarbageCollectionTest.java b/src/test/java/org/lmdbjava/GarbageCollectionTest.java
@@ -0,0 +1,111 @@
+/*-
+ * #%L
+ * LmdbJava
+ * %%
+ * Copyright (C) 2016 - 2023 The LmdbJava Open Source Project
+ * %%
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * 
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * #L%
+ */
+
+package org.lmdbjava;
+
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.mockito.MockedStatic;
+import org.mockito.Mockito;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+
+import static java.nio.ByteBuffer.allocateDirect;
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.Assert.fail;
+import static org.lmdbjava.DbiFlags.MDB_CREATE;
+import static org.lmdbjava.Env.create;
+
+@SuppressFBWarnings({"DM_GC", "RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT"})
+@SuppressWarnings("PMD.DoNotCallGarbageCollectionExplicitly")
+public class GarbageCollectionTest {
+
+    private static final String DB_NAME = "my DB";
+    private static final String KEY_PREFIX = "Uncorruptedkey";
+    private static final String VAL_PREFIX = "Uncorruptedval";
+
+    @Rule
+    public final TemporaryFolder tmp = new TemporaryFolder();
+    private void putBuffer(Dbi<ByteBuffer> db, Txn<ByteBuffer> txn, int i) {
+        ByteBuffer key = allocateDirect(24);
+        ByteBuffer val = allocateDirect(24);
+        key.put((KEY_PREFIX+i).getBytes(UTF_8)).flip();
+        val.put((VAL_PREFIX+i).getBytes(UTF_8)).flip();
+        db.put(txn, key, val);
+    }
+    @Test
+    public void buffersNotGarbageCollectedTest() throws IOException {
+        final File path = tmp.newFolder();
+
+        final Env<ByteBuffer> env = create()
+                .setMapSize(2_085_760_999)
+                .setMaxDbs(1)
+                .open(path);
+        final Dbi<ByteBuffer> db = env.openDbi(DB_NAME, MDB_CREATE);
+
+        // Trigger compilation and whatnot
+        try (Txn<ByteBuffer> txn = env.txnWrite()) {
+            for (int i = 0; i < 5000; i++) {
+                putBuffer(db, txn, i);
+            }
+            txn.commit();
+        }
+        // Call gc before writing to lmdb and after last reference to buffer by changing the behavior of mask
+        try (MockedStatic<MaskedFlag> mockedStatic = Mockito.mockStatic(MaskedFlag.class)) {
+            mockedStatic.when(MaskedFlag::mask).thenAnswer(invocationOnMock -> {
+                System.gc();
+                return 0;
+            });
+            try (Txn<ByteBuffer> txn = env.txnWrite()) {
+                for (int i = 0; i < 1000; i++) {
+                    putBuffer(db, txn, i);
+                }
+                txn.commit();
+            }
+        }
+
+        // Find corrupt keys
+        try (Txn<ByteBuffer> txn = env.txnRead()) {
+            try (Cursor<ByteBuffer> c = db.openCursor(txn)) {
+                if (c.first()) {
+                    do {
+                        byte[] rkey = new byte[c.key().remaining()];
+                        c.key().get(rkey);
+                        byte[] rval = new byte[c.val().remaining()];
+                        c.val().get(rval);
+                        String skey = new String(rkey, UTF_8);
+                        String sval = new String(rval, UTF_8);
+                        if (!skey.startsWith("Uncorruptedkey")) {
+                            fail("Found corrupt key " + skey);
+                        }
+                        if (!sval.startsWith("Uncorruptedval")) {
+                            fail("Found corrupt val " + sval);
+                        }
+                    } while (c.next());
+                }
+            }
+        }
+        env.close();
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -162,6 +162,7 @@ public boolean get(final T key, final T data, final SeekOp op) {`
`162`	`162`	`checkRc(rc);`
`163`	`163`	`kv.keyOut();`
`164`	`164`	`kv.valOut();`
	`165`	`+ ReferenceUtil.reachabilityFence0(key);`
`165`	`166`	`return true;`
`166`	`167`	`}`
`167`	`168`
`@@ -192,6 +193,7 @@ public boolean get(final T key, final GetOp op) {`
`192`	`193`	`checkRc(rc);`
`193`	`194`	`kv.keyOut();`
`194`	`195`	`kv.valOut();`
	`196`	`+ ReferenceUtil.reachabilityFence0(key);`
`195`	`197`	`return true;`
`196`	`198`	`}`
`197`	`199`
`@@ -266,6 +268,8 @@ public boolean put(final T key, final T val, final PutFlags... op) {`
`266`	`268`	`return false;`
`267`	`269`	`}`
`268`	`270`	`checkRc(rc);`
	`271`	`+ ReferenceUtil.reachabilityFence0(key);`
	`272`	`+ ReferenceUtil.reachabilityFence0(val);`
`269`	`273`	`return true;`
`270`	`274`	`}`
`271`	`275`
`@@ -304,6 +308,8 @@ public void putMultiple(final T key, final T val, final int elements,`
`304`	`308`	`final int rc = LIB.mdb_cursor_put(ptrCursor, txn.kv().pointerKey(),`
`305`	`309`	`dataPtr, mask);`
`306`	`310`	`checkRc(rc);`
	`311`	`+ ReferenceUtil.reachabilityFence0(key);`
	`312`	`+ ReferenceUtil.reachabilityFence0(val);`
`307`	`313`	`}`
`308`	`314`
`309`	`315`	`/**`
`@@ -362,6 +368,7 @@ public T reserve(final T key, final int size, final PutFlags... op) {`
`362`	`368`	`checkRc(LIB.mdb_cursor_put(ptrCursor, kv.pointerKey(), kv.pointerVal(),`
`363`	`369`	`flags));`
`364`	`370`	`kv.valOut();`
	`371`	`+ ReferenceUtil.reachabilityFence0(key);`
`365`	`372`	`return val();`
`366`	`373`	`}`
`367`	`374`
Original file line number	Diff line number	Diff line change
`@@ -171,6 +171,8 @@ public boolean delete(final Txn<T> txn, final T key, final T val) {`
`171`	`171`	`return false;`
`172`	`172`	`}`
`173`	`173`	`checkRc(rc);`
	`174`	`+ ReferenceUtil.reachabilityFence0(key);`
	`175`	`+ ReferenceUtil.reachabilityFence0(val);`
`174`	`176`	`return true;`
`175`	`177`	`}`
`176`	`178`
`@@ -239,6 +241,7 @@ public T get(final Txn<T> txn, final T key) {`
`239`	`241`	`return null;`
`240`	`242`	`}`
`241`	`243`	`checkRc(rc);`
	`244`	`+ ReferenceUtil.reachabilityFence0(key);`
`242`	`245`	`return txn.kv().valOut(); // marked as out in LMDB C docs`
`243`	`246`	`}`
`244`	`247`
`@@ -386,6 +389,8 @@ public boolean put(final Txn<T> txn, final T key, final T val,`
`386`	`389`	`return false;`
`387`	`390`	`}`
`388`	`391`	`checkRc(rc);`
	`392`	`+ ReferenceUtil.reachabilityFence0(key);`
	`393`	`+ ReferenceUtil.reachabilityFence0(val);`
`389`	`394`	`return true;`
`390`	`395`	`}`
`391`	`396`
`@@ -421,6 +426,7 @@ public T reserve(final Txn<T> txn, final T key, final int size,`
`421`	`426`	`checkRc(LIB.mdb_put(txn.pointer(), ptr, txn.kv().pointerKey(), txn.kv()`
`422`	`427`	`.pointerVal(), flags));`
`423`	`428`	`txn.kv().valOut(); // marked as in,out in LMDB C docs`
	`429`	`+ ReferenceUtil.reachabilityFence0(key);`
`424`	`430`	`return txn.val();`
`425`	`431`	`}`
`426`	`432`