From 0c636f1a2718de9d3e01c8afd04699a162265539 Mon Sep 17 00:00:00 2001 From: Ian Kang Date: Thu, 23 Mar 2023 16:16:16 -0400 Subject: [PATCH] Hold strong reference to all buffers passed to LmdbJava (fixes #207) --- pom.xml | 8 ++ src/main/java/org/lmdbjava/Cursor.java | 7 ++ src/main/java/org/lmdbjava/Dbi.java | 6 + src/main/java/org/lmdbjava/ReferenceUtil.java | 55 +++++++++ .../org/lmdbjava/GarbageCollectionTest.java | 111 ++++++++++++++++++ 5 files changed, 187 insertions(+) create mode 100644 src/main/java/org/lmdbjava/ReferenceUtil.java create mode 100644 src/test/java/org/lmdbjava/GarbageCollectionTest.java diff --git a/pom.xml b/pom.xml index 09b36ed..fa02502 100644 --- a/pom.xml +++ b/pom.xml @@ -88,6 +88,12 @@ 0.9.29-1 true + + org.mockito + mockito-inline + 4.11.0 + test + @@ -110,6 +116,8 @@ com.github.jnr:jffi + org.mockito:mockito-core + org.mockito:mockito-inline diff --git a/src/main/java/org/lmdbjava/Cursor.java b/src/main/java/org/lmdbjava/Cursor.java index aa98a9e..ed7c184 100644 --- 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 index d0eb0a1..ef8ec31 100644 --- 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 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 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 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 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 new file mode 100644 index 0000000..0002a50 --- /dev/null +++ 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 + * + * http://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 + * strongly reachable, + * 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. + * + *

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. + * + *

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. + * It is the caller's responsibility to ensure that this synchronization will not cause deadlock. + * + * @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 new file mode 100644 index 0000000..aa8a885 --- /dev/null +++ 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 + * + * http://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 db, Txn 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 env = create() + .setMapSize(2_085_760_999) + .setMaxDbs(1) + .open(path); + final Dbi db = env.openDbi(DB_NAME, MDB_CREATE); + + // Trigger compilation and whatnot + try (Txn 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 mockedStatic = Mockito.mockStatic(MaskedFlag.class)) { + mockedStatic.when(MaskedFlag::mask).thenAnswer(invocationOnMock -> { + System.gc(); + return 0; + }); + try (Txn txn = env.txnWrite()) { + for (int i = 0; i < 1000; i++) { + putBuffer(db, txn, i); + } + txn.commit(); + } + } + + // Find corrupt keys + try (Txn txn = env.txnRead()) { + try (Cursor 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(); + } +}