diff --git a/src/main/java/net/openhft/chronicle/map/ReplicatedChronicleMap.java b/src/main/java/net/openhft/chronicle/map/ReplicatedChronicleMap.java index 01358a276..4d899f503 100644 --- a/src/main/java/net/openhft/chronicle/map/ReplicatedChronicleMap.java +++ b/src/main/java/net/openhft/chronicle/map/ReplicatedChronicleMap.java @@ -273,7 +273,7 @@ final class ReplicatedChronicleMap, } @Override - T lookupUsing(K key, V value, LockType lockTypeType) { + T lookupUsing(K key, V usingValue, LockType lockType, boolean mustReuseValue) { checkKey(key); ThreadLocalCopies copies = keyInteropProvider.getCopies(null); KI keyWriter = keyInteropProvider.get(copies, originalKeyInterop); @@ -284,24 +284,26 @@ final class ReplicatedChronicleMap, int segmentNum = getSegment(hash); long segmentHash = segmentHash(hash); - boolean create = lockTypeType == LockType.WRITE_LOCK; + boolean create = lockType == LockType.WRITE_LOCK; Segment segment = segment(segmentNum); - MutableLockedEntry lock = (lockTypeType == LockType.WRITE_LOCK) + MutableLockedEntry lock = (lockType == LockType.WRITE_LOCK) ? segment.writeLock() : segment.readLock(); V v = segment.acquireWithoutLock(copies, metaKeyWriter, keyWriter, key, - value, + usingValue, segmentHash, create, create ? timeProvider.currentTimeMillis() : 0L); + checkReallyUsingValue(usingValue, mustReuseValue, v); + lock.copies = copies; lock.metaKeyInterop = metaKeyWriter; lock.keyInterop = keyWriter; lock.segmentHash = segmentHash; - if (!isNativeValueClass && lockTypeType == LockType.WRITE_LOCK && v == null) - v = value; + if (!isNativeValueClass && lockType == LockType.WRITE_LOCK && v == null) + v = usingValue; lock.value(v); lock.key(key); diff --git a/src/main/java/net/openhft/chronicle/map/VanillaChronicleMap.java b/src/main/java/net/openhft/chronicle/map/VanillaChronicleMap.java index 749e0dfc2..051520842 100644 --- a/src/main/java/net/openhft/chronicle/map/VanillaChronicleMap.java +++ b/src/main/java/net/openhft/chronicle/map/VanillaChronicleMap.java @@ -25,8 +25,6 @@ import net.openhft.chronicle.hash.serialization.SizeMarshaller; import net.openhft.chronicle.hash.serialization.internal.MetaBytesInterop; import net.openhft.chronicle.hash.serialization.internal.MetaBytesWriter; import net.openhft.chronicle.hash.serialization.internal.MetaProvider; -import net.openhft.chronicle.map.MultiMap; -import net.openhft.chronicle.map.MultiMapFactory; import net.openhft.lang.collection.DirectBitSet; import net.openhft.lang.collection.SingleThreadedDirectBitSet; import net.openhft.lang.io.*; @@ -357,7 +355,7 @@ class VanillaChronicleMap, public V get(Object key) { try (Context entry = lookupUsing((K) key, null, - LockType.READ_LOCK)) { + LockType.READ_LOCK, false)) { return entry.value(); } } @@ -366,7 +364,7 @@ class VanillaChronicleMap, public V getUsing(K key, V usingValue) { try (Context entry = lookupUsing(key, usingValue, - LockType.READ_LOCK)) { + LockType.READ_LOCK, false)) { return entry.value(); } } @@ -374,20 +372,21 @@ class VanillaChronicleMap, @NotNull @Override public WriteContext acquireUsingLocked(@NotNull K key, @NotNull V usingValue) { - return this.lookupUsing(key, usingValue, LockType.WRITE_LOCK); + return this.lookupUsing(key, usingValue, LockType.WRITE_LOCK, true); } @NotNull @Override public ReadContext getUsingLocked(@NotNull K key, @NotNull V usingValue) { - return this.lookupUsing(key, usingValue, LockType.READ_LOCK); + return this.lookupUsing(key, usingValue, LockType.READ_LOCK, true); } @Override public V acquireUsing(@NotNull K key, @NotNull V usingValue) { - try (WriteContext kvContext = lookupUsing(key, usingValue, LockType.WRITE_LOCK)) { + try (WriteContext kvContext = + lookupUsing(key, usingValue, LockType.WRITE_LOCK, false)) { if (!isNativeValueClass) kvContext.dontPutOnClose(); @@ -397,7 +396,7 @@ class VanillaChronicleMap, } - T lookupUsing(K key, V value, LockType lockTypeType) { + T lookupUsing(K key, V usingValue, LockType lockType, boolean mustReuseValue) { checkKey(key); ThreadLocalCopies copies = keyInteropProvider.getCopies(null); KI keyInterop = keyInteropProvider.get(copies, originalKeyInterop); @@ -409,20 +408,22 @@ class VanillaChronicleMap, long segmentHash = segmentHash(hash); Segment segment = segments[segmentNum]; - MutableLockedEntry lock = (lockTypeType == LockType.WRITE_LOCK) + MutableLockedEntry lock = (lockType == LockType.WRITE_LOCK) ? segment.writeLock() : segment.readLock(); - V v = segment.acquireWithoutLock(copies, metaKeyInterop, keyInterop, key, value, - segmentHash, lockTypeType == LockType.WRITE_LOCK); + V v = segment.acquireWithoutLock(copies, metaKeyInterop, keyInterop, key, usingValue, + segmentHash, lockType == LockType.WRITE_LOCK); + + checkReallyUsingValue(usingValue, mustReuseValue, v); lock.copies = copies; lock.metaKeyInterop = metaKeyInterop; lock.keyInterop = keyInterop; lock.segmentHash = segmentHash; - if (!isNativeValueClass && lockTypeType == LockType.WRITE_LOCK && v == null) - v = value; + if (!isNativeValueClass && lockType == LockType.WRITE_LOCK && v == null) + v = usingValue; lock.value(v); lock.key(key); @@ -431,6 +432,12 @@ class VanillaChronicleMap, } + void checkReallyUsingValue(V usingValue, boolean mustReuseValue, V returnedValue) { + if (mustReuseValue && returnedValue != null && returnedValue != usingValue) + throw new IllegalArgumentException("acquireUsingLocked/getUsingLocked MUST reuse given " + + "values. Given value" + usingValue + " cannot be reused to read " + returnedValue); + } + @Override public boolean containsKey(final Object k) { checkKey(k); @@ -849,17 +856,7 @@ class VanillaChronicleMap, } V acquireWithoutLock(ThreadLocalCopies copies, MKI metaKeyInterop, KI keyInterop, K key, V usingValue, - long hash2, boolean create) { - V v = acquire0(copies, metaKeyInterop, keyInterop, key, usingValue, hash2, false); - if (create && v == null) - v = acquire0(copies, metaKeyInterop, keyInterop, key, usingValue, hash2, true); - return v; - } - - V acquire0(ThreadLocalCopies copies, MKI metaKeyInterop, KI keyInterop, K key, V usingValue, long hash2, boolean create) { - - long keySize = metaKeyInterop.size(keyInterop, key); MultiStoreBytes entry = acquireTmpBytes(); long offset = searchKey(keyInterop, metaKeyInterop, key, keySize, hash2, entry, diff --git a/src/test/java/net/openhft/chronicle/map/ChronicleMapTest.java b/src/test/java/net/openhft/chronicle/map/ChronicleMapTest.java index 098f16f25..f4310763b 100644 --- a/src/test/java/net/openhft/chronicle/map/ChronicleMapTest.java +++ b/src/test/java/net/openhft/chronicle/map/ChronicleMapTest.java @@ -1362,7 +1362,6 @@ public class ChronicleMapTest { // check that the entry was removed try (ReadContext context = map.getUsingLocked("one", value)) { - assert value == context.value(); assertEquals(false, context.present()); assertEquals(null, context.value()); } @@ -1389,7 +1388,7 @@ public class ChronicleMapTest { } - @Test(expected = IllegalAccessException.class) + @Test(expected = IllegalStateException.class) public void testAcquireUsingLockedWithString() throws IOException { ChronicleMapBuilder builder = ChronicleMapBuilder