From 9438b1ea5bd53f922ae2a2fa320b130bf3fbeafe Mon Sep 17 00:00:00 2001 From: Dmitry Pisklov Date: Tue, 26 May 2020 22:15:33 +0100 Subject: [PATCH 1/7] implement AbstractCloseable - OpenHFT/Chronicle-Core#116 --- .../net/openhft/chronicle/hash/HashSegmentContext.java | 5 +++++ .../chronicle/hash/impl/VanillaChronicleHash.java | 9 ++++----- .../chronicle/map/OldDeletedEntriesCleanupThread.java | 5 +++++ .../chronicle/map/impl/stage/query/AcquireHandle.java | 5 +++-- src/main/java/net/openhft/chronicle/set/SetFromMap.java | 5 +++++ .../openhft/chronicle/map/ReplicationCheckingMap.java | 5 +++++ 6 files changed, 27 insertions(+), 7 deletions(-) diff --git a/src/main/java/net/openhft/chronicle/hash/HashSegmentContext.java b/src/main/java/net/openhft/chronicle/hash/HashSegmentContext.java index fa3fb1280..8fbdfe8b6 100644 --- a/src/main/java/net/openhft/chronicle/hash/HashSegmentContext.java +++ b/src/main/java/net/openhft/chronicle/hash/HashSegmentContext.java @@ -57,4 +57,9 @@ public interface HashSegmentContext> extends HashConte * Returns the number of present entries in this segment. */ long size(); + + @Override + default boolean isClosed() { + throw new UnsupportedOperationException(); + } } 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 1d613653b..ad8756bbe 100755 --- a/src/main/java/net/openhft/chronicle/hash/impl/VanillaChronicleHash.java +++ b/src/main/java/net/openhft/chronicle/hash/impl/VanillaChronicleHash.java @@ -22,6 +22,7 @@ import net.openhft.chronicle.bytes.MappedBytesStoreFactory; import net.openhft.chronicle.bytes.NativeBytesStore; import net.openhft.chronicle.core.Maths; import net.openhft.chronicle.core.OS; +import net.openhft.chronicle.core.io.AbstractCloseable; import net.openhft.chronicle.hash.*; import net.openhft.chronicle.hash.impl.util.BuildVersion; import net.openhft.chronicle.hash.impl.util.Cleaner; @@ -38,7 +39,6 @@ import net.openhft.chronicle.map.ChronicleMapBuilder; import net.openhft.chronicle.values.Values; import net.openhft.chronicle.wire.Marshallable; import net.openhft.chronicle.wire.WireIn; -import net.openhft.chronicle.wire.WireInternal; import net.openhft.chronicle.wire.WireOut; import org.jetbrains.annotations.NotNull; @@ -65,9 +65,8 @@ import static net.openhft.chronicle.hash.impl.CompactOffHeapLinearHashTable.*; import static net.openhft.chronicle.map.ChronicleHashCorruptionImpl.format; import static net.openhft.chronicle.map.ChronicleHashCorruptionImpl.report; -public abstract class VanillaChronicleHash, SC extends HashSegmentContext, - ECQ extends ExternalHashQueryContext> +public abstract class VanillaChronicleHash, SC extends HashSegmentContext, ECQ extends ExternalHashQueryContext> + extends AbstractCloseable implements ChronicleHash, Marshallable { public static final long TIER_COUNTERS_AREA_SIZE = 64; @@ -660,7 +659,7 @@ public abstract class VanillaChronicleHash implements Closeable { +public class AcquireHandle extends AbstractCloseable { @StageRef CheckOnEachPublicOperation checkOnEachPublicOperation; @@ -33,7 +34,7 @@ public class AcquireHandle implements Closeable { UsingReturnValue usingReturn; @Override - public void close() { + protected void performClose() { checkOnEachPublicOperation.checkOnEachPublicOperation(); q.replaceValue(q.entry(), q.wrapValueAsData(usingReturn.returnValue())); q.close(); diff --git a/src/main/java/net/openhft/chronicle/set/SetFromMap.java b/src/main/java/net/openhft/chronicle/set/SetFromMap.java index c31204622..fb1c049d4 100644 --- a/src/main/java/net/openhft/chronicle/set/SetFromMap.java +++ b/src/main/java/net/openhft/chronicle/set/SetFromMap.java @@ -203,6 +203,11 @@ class SetFromMap extends AbstractSet implements ChronicleSet { m.close(); } + @Override + public boolean isClosed() { + return m.isClosed(); + } + @Override public boolean isOpen() { return m.isOpen(); diff --git a/src/test/java/net/openhft/chronicle/map/ReplicationCheckingMap.java b/src/test/java/net/openhft/chronicle/map/ReplicationCheckingMap.java index 910662f8d..f7902816d 100644 --- a/src/test/java/net/openhft/chronicle/map/ReplicationCheckingMap.java +++ b/src/test/java/net/openhft/chronicle/map/ReplicationCheckingMap.java @@ -331,6 +331,11 @@ public class ReplicationCheckingMap implements ChronicleMap { map2.close(); } + @Override + public boolean isClosed() { + return false; + } + @Override public boolean isOpen() { throw new UnsupportedOperationException(); From 21b2676a3c73b5d82ea2d538aada604f349c6e88 Mon Sep 17 00:00:00 2001 From: Dmitry Pisklov Date: Thu, 28 May 2020 16:37:52 +0100 Subject: [PATCH 2/7] Small fixes --- .../hash/impl/util/ThrowablesTest.java | 1 + .../map/AbstractMarshallableKeyValueTest.java | 3 ++- .../net/openhft/lang/values/DoubleArray.java | 12 ++++------ .../openhft/lang/values/DoubleArrayTest.java | 23 ------------------- 4 files changed, 7 insertions(+), 32 deletions(-) diff --git a/src/test/java/net/openhft/chronicle/hash/impl/util/ThrowablesTest.java b/src/test/java/net/openhft/chronicle/hash/impl/util/ThrowablesTest.java index 8e238a7d2..9826d1fc2 100644 --- a/src/test/java/net/openhft/chronicle/hash/impl/util/ThrowablesTest.java +++ b/src/test/java/net/openhft/chronicle/hash/impl/util/ThrowablesTest.java @@ -48,6 +48,7 @@ public class ThrowablesTest { Throwables.propagateNotWrapping(null, IOException.class); } + @SuppressWarnings("RedundantThrows") @Test(expected = NullPointerException.class) public void testPropagateNotWrappingNullWrappingType() throws Throwable { Throwables.propagateNotWrapping(new IOException(), null); diff --git a/src/test/java/net/openhft/chronicle/map/AbstractMarshallableKeyValueTest.java b/src/test/java/net/openhft/chronicle/map/AbstractMarshallableKeyValueTest.java index d22d6731b..3abfdd30b 100644 --- a/src/test/java/net/openhft/chronicle/map/AbstractMarshallableKeyValueTest.java +++ b/src/test/java/net/openhft/chronicle/map/AbstractMarshallableKeyValueTest.java @@ -20,6 +20,7 @@ import net.openhft.chronicle.wire.SelfDescribingMarshallable; import org.junit.Test; import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; public final class AbstractMarshallableKeyValueTest { @@ -31,7 +32,7 @@ public final class AbstractMarshallableKeyValueTest { map.put(new Key(), new Value()); - assertThat(map.get(new Key()).number, is(new Value().number)); + assertEquals(new Value().number, map.get(new Key()).number); } private static final class Key extends SelfDescribingMarshallable { diff --git a/src/test/java/net/openhft/lang/values/DoubleArray.java b/src/test/java/net/openhft/lang/values/DoubleArray.java index d080f305d..be85dcbd5 100755 --- a/src/test/java/net/openhft/lang/values/DoubleArray.java +++ b/src/test/java/net/openhft/lang/values/DoubleArray.java @@ -22,14 +22,10 @@ import net.openhft.chronicle.bytes.DynamicallySized; import net.openhft.chronicle.bytes.NativeBytesStore; import net.openhft.chronicle.values.Copyable; -/** - * Created by peter.lawrey on 23/04/2015. - */ public class DoubleArray implements Byteable, Copyable, DynamicallySized { - static boolean HACK = true; - private static int CAPACITY = 0; // assume a 32-bit size. - private static int LENGTH = CAPACITY + 4; // assume a 32-bit size. - private static int BASE = LENGTH + 4; + private final static int CAPACITY = 0; // assume a 32-bit size. + private final static int LENGTH = CAPACITY + 4; // assume a 32-bit size. + private final static int BASE = LENGTH + 4; private final int capacity; private BytesStore bs; @@ -64,7 +60,7 @@ public class DoubleArray implements Byteable, Copyable, Dynamically } public int length() { - return HACK && bs == null ? 6 * 8 : bs.readInt(LENGTH + offset); + return bs == null ? 6 * 8 : bs.readInt(LENGTH + offset); } public int capacity() { diff --git a/src/test/java/net/openhft/lang/values/DoubleArrayTest.java b/src/test/java/net/openhft/lang/values/DoubleArrayTest.java index 2c91b0e35..6c54051aa 100755 --- a/src/test/java/net/openhft/lang/values/DoubleArrayTest.java +++ b/src/test/java/net/openhft/lang/values/DoubleArrayTest.java @@ -85,27 +85,4 @@ public class DoubleArrayTest { System.out.println(proxyMap.get(2)); proxyMap.close(); } - - @Test - @Ignore("TODO What is HACK???") - public void addToAMap2() { - DoubleArray.HACK = false; - DoubleArray a = new DoubleArray(10); - a.setData(new double[]{1, 2, 3, 4, 5}); - - DoubleArray b = new DoubleArray(10); - b.setData(new double[]{5, 6, 7, 8, 9}); - - ChronicleMap proxyMap = ChronicleMapBuilder - .of(Integer.class, DoubleArray.class) - .averageValueSize(6 * 8) - .create(); - proxyMap.put(1, a); - proxyMap.put(2, b); - - System.out.println(proxyMap.get(1)); - System.out.println(proxyMap.get(2)); - proxyMap.close(); - DoubleArray.HACK = true; - } } \ No newline at end of file From d7c318986bafe00dd5d743e9be0fd8e7c5be741b Mon Sep 17 00:00:00 2001 From: Dmitry Pisklov Date: Thu, 28 May 2020 16:54:28 +0100 Subject: [PATCH 3/7] fix longSize() implementation for closed map --- .../openhft/chronicle/map/VanillaChronicleMap.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/main/java/net/openhft/chronicle/map/VanillaChronicleMap.java b/src/main/java/net/openhft/chronicle/map/VanillaChronicleMap.java index 1c33ded38..23dda6548 100755 --- a/src/main/java/net/openhft/chronicle/map/VanillaChronicleMap.java +++ b/src/main/java/net/openhft/chronicle/map/VanillaChronicleMap.java @@ -131,7 +131,7 @@ public class VanillaChronicleMap alignment = wireIn.read(() -> "alignment").int32(); worstAlignment = wireIn.read(() -> "worstAlignment").int32(); - maxBloatFactor = wireIn.read(() -> "maxBloatFactor").float64(); + maxBloatFactor = wireIn.read(() -> "maxBloatFactor").float64(); } @Override @@ -203,12 +203,13 @@ public class VanillaChronicleMap @Override public final long longSize() { long result = 0L; - try (IterationContext c = iterationContext()) { - for (int segmentIndex = 0; segmentIndex < segments(); segmentIndex++) { - c.initSegmentIndex(segmentIndex); - result += c.size(); + if (!isClosed()) + try (IterationContext c = iterationContext()) { + for (int segmentIndex = 0; segmentIndex < segments(); segmentIndex++) { + c.initSegmentIndex(segmentIndex); + result += c.size(); + } } - } return result; } From b0ef26bb582b7511fe71173ee74df080f043d5de Mon Sep 17 00:00:00 2001 From: Dmitry Pisklov Date: Thu, 28 May 2020 17:06:58 +0100 Subject: [PATCH 4/7] fix longSize() implementation for closed map --- .../java/net/openhft/chronicle/map/VanillaChronicleMap.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/net/openhft/chronicle/map/VanillaChronicleMap.java b/src/main/java/net/openhft/chronicle/map/VanillaChronicleMap.java index 23dda6548..a49f79934 100755 --- a/src/main/java/net/openhft/chronicle/map/VanillaChronicleMap.java +++ b/src/main/java/net/openhft/chronicle/map/VanillaChronicleMap.java @@ -203,7 +203,7 @@ public class VanillaChronicleMap @Override public final long longSize() { long result = 0L; - if (!isClosed()) + if (!isClosed() && this.cxt != null) try (IterationContext c = iterationContext()) { for (int segmentIndex = 0; segmentIndex < segments(); segmentIndex++) { c.initSegmentIndex(segmentIndex); From fb3b953b9d24451a1e7541831e0f63f2c5f38e3b Mon Sep 17 00:00:00 2001 From: Dmitry Pisklov Date: Thu, 28 May 2020 20:01:51 +0100 Subject: [PATCH 5/7] fix longSize() implementation - fix the test --- .../chronicle/hash/impl/VanillaChronicleHash.java | 7 ++++++- .../net/openhft/chronicle/map/MapCloseTest.java | 13 +++++++------ 2 files changed, 13 insertions(+), 7 deletions(-) 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 ad8756bbe..a42226f62 100755 --- a/src/main/java/net/openhft/chronicle/hash/impl/VanillaChronicleHash.java +++ b/src/main/java/net/openhft/chronicle/hash/impl/VanillaChronicleHash.java @@ -679,7 +679,12 @@ public abstract class VanillaChronicleHash, SC extends @Override public boolean isOpen() { - return !resources.closed(); + return !isClosed(); + } + + @Override + public boolean isClosed() { + return super.isClosed() || resources.closed(); } public final void checkKey(Object key) { diff --git a/src/test/java/net/openhft/chronicle/map/MapCloseTest.java b/src/test/java/net/openhft/chronicle/map/MapCloseTest.java index b99e6e957..767044fb8 100644 --- a/src/test/java/net/openhft/chronicle/map/MapCloseTest.java +++ b/src/test/java/net/openhft/chronicle/map/MapCloseTest.java @@ -18,13 +18,14 @@ package net.openhft.chronicle.map; import net.openhft.chronicle.hash.ChronicleHashClosedException; import net.openhft.chronicle.hash.impl.stage.hash.ChainingInterface; -import org.junit.Assert; import org.junit.Test; import java.util.concurrent.CountDownLatch; import java.util.concurrent.Semaphore; import static net.openhft.chronicle.hash.impl.BigSegmentHeader.LOCK_TIMEOUT_SECONDS; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertSame; public class MapCloseTest { @@ -58,7 +59,7 @@ public class MapCloseTest { }); } - @Test(expected = ChronicleHashClosedException.class) + @Test public void testSizeAfterCloseThrowsChronicleHashClosedException() throws InterruptedException { ChronicleMap map = @@ -66,7 +67,7 @@ public class MapCloseTest { Thread t = new Thread(() -> map.close()); t.start(); t.join(); - map.size(); + assertEquals(0, map.size()); } @Test(expected = RuntimeException.class) @@ -117,14 +118,14 @@ public class MapCloseTest { t1.start(); t2.start(); latch.await(); - Assert.assertEquals(2, map.allContexts().size()); + assertEquals(2, map.allContexts().size()); semaphore.release(2); t1.join(); t2.join(); map.get(1); - Assert.assertEquals(1, map.allContexts().size()); + assertEquals(1, map.allContexts().size()); ChainingInterface cxt = map.allContexts().get(0).get().get(); - Assert.assertTrue(cxt == map.queryContext(1)); + assertSame(cxt, map.queryContext(1)); } } From e56fb15f9506d9d2b46bd3f64683f41915b8e91c Mon Sep 17 00:00:00 2001 From: Rob Austin Date: Fri, 29 May 2020 13:26:43 +0100 Subject: [PATCH 6/7] fixed java.lang.NullPointerException at net.openhft.chronicle.hash.impl.VanillaChronicleHash.isClosed(VanillaChronicleHash.java:687) at net.openhft.chronicle.map.VanillaChronicleMap.longSize(VanillaChronicleMap.java:206) at net.openhft.chronicle.map.AbstractChronicleMap.mapEquals(AbstractChronicleMap.java:183) at net.openhft.chronicle.map.VanillaChronicleMap.equals(VanillaChronicleMap.java:321) at java.util.WeakHashMap.eq(WeakHashMap.java:287) at java.util.WeakHashMap.put(WeakHashMap.java:454) at java.util.Collections$SynchronizedMap.put(Collections.java:2590) at net.openhft.chronicle.core.io.AbstractCloseable.(AbstractCloseable.java:46) at net.openhft.chronicle.hash.impl.VanillaChronicleHash.(VanillaChronicleHash.java:156) --- .../chronicle/hash/impl/VanillaChronicleHash.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) 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 a42226f62..dd01ed81e 100755 --- a/src/main/java/net/openhft/chronicle/hash/impl/VanillaChronicleHash.java +++ b/src/main/java/net/openhft/chronicle/hash/impl/VanillaChronicleHash.java @@ -74,10 +74,9 @@ public abstract class VanillaChronicleHash, SC extends // --- Start of instance fields --- /** - * Global mutable state lock doesn't yet need read-write levels and waits; - * Used the same locking strategy as in segment locks - * (VanillaReadWriteUpdateWithWaitsLockingStrategy) in order to simplify Chronicle Map - * specification (having only one kind of locks to specify and implement). + * Global mutable state lock doesn't yet need read-write levels and waits; Used the same locking strategy as in segment locks + * (VanillaReadWriteUpdateWithWaitsLockingStrategy) in order to simplify Chronicle Map specification (having only one kind of locks to specify and + * implement). */ static final LockingStrategy globalMutableStateLockingStrategy = VanillaReadWriteUpdateWithWaitsLockingStrategy.instance(); @@ -684,7 +683,8 @@ public abstract class VanillaChronicleHash, SC extends @Override public boolean isClosed() { - return super.isClosed() || resources.closed(); + ChronicleHashResources resources = this.resources; + return super.isClosed() || (resources != null && resources.closed()); } public final void checkKey(Object key) { @@ -1055,8 +1055,8 @@ public abstract class VanillaChronicleHash, SC extends } /** - * {@link ChronicleHashCloseOnExitHook} needs to use {@code VanillaChronicleHash}es as - * WeakHashMap keys, but with identity comparison, not Map's equals() and hashCode(). + * {@link ChronicleHashCloseOnExitHook} needs to use {@code VanillaChronicleHash}es as WeakHashMap keys, but with identity comparison, not Map's + * equals() and hashCode(). */ public class Identity { public VanillaChronicleHash hash() { From b471a97a0d79b8743a4aea305aa001474127e1b1 Mon Sep 17 00:00:00 2001 From: Rob Austin Date: Fri, 29 May 2020 15:06:20 +0100 Subject: [PATCH 7/7] added default is closed --- .../openhft/chronicle/core/io/Closeable.java | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 src/main/java/net/openhft/chronicle/core/io/Closeable.java diff --git a/src/main/java/net/openhft/chronicle/core/io/Closeable.java b/src/main/java/net/openhft/chronicle/core/io/Closeable.java new file mode 100644 index 000000000..91b10ab38 --- /dev/null +++ b/src/main/java/net/openhft/chronicle/core/io/Closeable.java @@ -0,0 +1,47 @@ +package net.openhft.chronicle.core.io; + +import net.openhft.chronicle.core.Jvm; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.io.IOException; +import java.util.Collection; + +public interface Closeable extends java.io.Closeable, QueryCloseable { + + static void closeQuietly(@NotNull Object... closeables) { + closeQuietly((Object) closeables); + } + + static void closeQuietly(@Nullable Object o) { + if (o instanceof Collection) { + ((Collection) o).forEach(Closeable::closeQuietly); + } else if (o instanceof Object[]) { + for (Object o2 : (Object[]) o) { + closeQuietly(o2); + } + } else if (o instanceof java.io.Closeable) { + try { + ((java.io.Closeable) o).close(); + } catch (IOException | IllegalStateException e) { + Jvm.debug().on(Closeable.class, e); + } + } + } + + /** + * Doesn't throw a checked exception. + */ + @Override + void close(); + + @Deprecated + default void notifyClosing() { + // take an action before everything else closes. + } + + @Override + default boolean isClosed() { + throw new UnsupportedOperationException("todo"); + } +}