diff --git a/src/main/java/net/openhft/chronicle/hash/impl/VanillaChronicleHash.java b/src/main/java/net/openhft/chronicle/hash/impl/VanillaChronicleHash.java index 90f714158..482b577ad 100644 --- a/src/main/java/net/openhft/chronicle/hash/impl/VanillaChronicleHash.java +++ b/src/main/java/net/openhft/chronicle/hash/impl/VanillaChronicleHash.java @@ -657,15 +657,20 @@ public abstract class VanillaChronicleHash cxt = new ThreadLocal<>(); } + @Override + protected void cleanupOnClose() { + super.cleanupOnClose(); + // Make GC life easier + valueReader = null; + valueDataAccess = null; + cxt = null; + } + public final V checkValue(Object value) { if (!valueClass.isInstance(value)) { throw new ClassCastException(toIdentityString() + ": Value must be a " + @@ -297,6 +306,9 @@ public class VanillaChronicleMap } final ChainingInterface q() { + ThreadLocal cxt = this.cxt; + if (cxt == null) + throw new ChronicleHashClosedException(this); ContextHolder contextHolder = cxt.get(); if (contextHolder == null) { ChainingInterface queryContext = newQueryContext(); @@ -327,6 +339,9 @@ public class VanillaChronicleMap } final ChainingInterface i() { + ThreadLocal cxt = this.cxt; + if (cxt == null) + throw new ChronicleHashClosedException(this); ContextHolder contextHolder = cxt.get(); if (contextHolder == null) { ChainingInterface iterationContext = newIterationContext(); diff --git a/src/test/java/net/openhft/chronicle/map/MemoryLeaksTest.java b/src/test/java/net/openhft/chronicle/map/MemoryLeaksTest.java index 3e5b98728..6009272ba 100644 --- a/src/test/java/net/openhft/chronicle/map/MemoryLeaksTest.java +++ b/src/test/java/net/openhft/chronicle/map/MemoryLeaksTest.java @@ -23,7 +23,6 @@ import net.openhft.chronicle.core.OS; import net.openhft.chronicle.core.values.IntValue; import net.openhft.chronicle.hash.serialization.impl.StringSizedReader; import net.openhft.chronicle.hash.serialization.impl.StringUtf8DataAccess; -import net.openhft.chronicle.map.impl.QueryContextInterface; import net.openhft.chronicle.values.Values; import org.junit.Assert; import org.junit.Before; @@ -151,21 +150,6 @@ public class MemoryLeaksTest { } assertEquals(nativeMemoryUsedBeforeMap, nativeMemoryUsed()); // Wait until chronicle map context (hence serializers) is collected by the GC - for (int i = 0; i < 6_000; i++) { - if (serializerCount.get() == serializersBeforeMap + 1) - break; - System.gc(); - byte[] garbage = new byte[10_000_000]; - Thread.sleep(10); - } - Assert.assertEquals(serializersBeforeMap + 1, serializerCount.get()); - // This assertion ensures GC doesn't reclaim the map before or during the loop iteration - // above, to ensure that we test that the direct memory and contexts are released because - // of the manual map.close(), despite the "leak" of the map object itself. - Assert.assertEquals(0, map.offHeapMemoryUsed()); - map = null; - // After cleaning the reference to the map, the last serializer, which was held in it's - // field, should be cleaned as well for (int i = 0; i < 6_000; i++) { if (serializerCount.get() == serializersBeforeMap) break; @@ -173,7 +157,11 @@ public class MemoryLeaksTest { byte[] garbage = new byte[10_000_000]; Thread.sleep(10); } - Assert.assertEquals(serializersBeforeMap, serializerCount.get()); + Assert.assertTrue(serializerCount.get() == serializersBeforeMap); + // This assertion ensures GC doesn't reclaim the map before or during the loop iteration + // above, to ensure that we test that the direct memory and contexts are released because + // of the manual map.close(), despite the "leak" of the map object itself. + Assert.assertEquals(0, map.offHeapMemoryUsed()); } private ChronicleMap getMap() throws IOException {