From e9e27c207a3541604077e6912df99a6eda5f4e86 Mon Sep 17 00:00:00 2001 From: Jaromir Hamala Date: Wed, 11 May 2016 22:11:13 +0300 Subject: [PATCH] Multiple IAtomicReference issues fixed 1. getAndAlter test was testing alterAndGet method 2. AlterAndGetOperation optimized - sen backup only when (serialized) new value is different from the original value 3. AlterOperation fixed - use serialized values to make a decision whether to persist change - otherwise a change is lost when the function mutates the original object instead of creating a backup 4. GetAndAlterOperation fixed - the same issues as with AlterOperation Fixes #8149 (cherry picked from commit 2ad2e7b) --- .../operations/AlterAndGetOperation.java | 12 ++++++---- .../operations/AlterOperation.java | 8 ++++--- .../operations/GetAndAlterOperation.java | 9 +++---- .../AtomicReferenceAbstractTest.java | 24 +++++++++++++++++++ 4 files changed, 42 insertions(+), 11 deletions(-) diff --git a/hazelcast/src/main/java/com/hazelcast/concurrent/atomicreference/operations/AlterAndGetOperation.java b/hazelcast/src/main/java/com/hazelcast/concurrent/atomicreference/operations/AlterAndGetOperation.java index d5478a6bfb49..6c64253b414b 100644 --- a/hazelcast/src/main/java/com/hazelcast/concurrent/atomicreference/operations/AlterAndGetOperation.java +++ b/hazelcast/src/main/java/com/hazelcast/concurrent/atomicreference/operations/AlterAndGetOperation.java @@ -37,12 +37,16 @@ public void run() throws Exception { IFunction f = nodeEngine.toObject(function); AtomicReferenceContainer atomicReferenceContainer = getReferenceContainer(); - Object input = nodeEngine.toObject(atomicReferenceContainer.get()); + Data originalData = atomicReferenceContainer.get(); + Object input = nodeEngine.toObject(originalData); //noinspection unchecked Object output = f.apply(input); - shouldBackup = true; - backup = nodeEngine.toData(output); - atomicReferenceContainer.set(backup); + Data serializedOutput = nodeEngine.toData(output); + shouldBackup = !isEquals(originalData, serializedOutput); + if (shouldBackup) { + backup = serializedOutput; + atomicReferenceContainer.set(backup); + } response = output; } diff --git a/hazelcast/src/main/java/com/hazelcast/concurrent/atomicreference/operations/AlterOperation.java b/hazelcast/src/main/java/com/hazelcast/concurrent/atomicreference/operations/AlterOperation.java index 3583f6298450..06988f0caf2d 100644 --- a/hazelcast/src/main/java/com/hazelcast/concurrent/atomicreference/operations/AlterOperation.java +++ b/hazelcast/src/main/java/com/hazelcast/concurrent/atomicreference/operations/AlterOperation.java @@ -37,12 +37,14 @@ public void run() throws Exception { IFunction f = nodeEngine.toObject(function); AtomicReferenceContainer reference = getReferenceContainer(); - Object input = nodeEngine.toObject(reference.get()); + Data originalData = reference.get(); + Object input = nodeEngine.toObject(originalData); //noinspection unchecked Object output = f.apply(input); - shouldBackup = !isEquals(input, output); + Data serializedOutput = nodeEngine.toData(output); + shouldBackup = !isEquals(originalData, serializedOutput); if (shouldBackup) { - backup = nodeEngine.toData(output); + backup = serializedOutput; reference.set(backup); } } diff --git a/hazelcast/src/main/java/com/hazelcast/concurrent/atomicreference/operations/GetAndAlterOperation.java b/hazelcast/src/main/java/com/hazelcast/concurrent/atomicreference/operations/GetAndAlterOperation.java index 72777e3a46c0..46926850d646 100644 --- a/hazelcast/src/main/java/com/hazelcast/concurrent/atomicreference/operations/GetAndAlterOperation.java +++ b/hazelcast/src/main/java/com/hazelcast/concurrent/atomicreference/operations/GetAndAlterOperation.java @@ -37,14 +37,15 @@ public void run() throws Exception { IFunction f = nodeEngine.toObject(function); AtomicReferenceContainer atomicReferenceContainer = getReferenceContainer(); + response = atomicReferenceContainer.get(); Object input = nodeEngine.toObject(atomicReferenceContainer.get()); - response = input; //noinspection unchecked Object output = f.apply(input); - shouldBackup = !isEquals(input, output); + Data serializedOutput = nodeEngine.toData(output); + shouldBackup = !isEquals(response, serializedOutput); if (shouldBackup) { - backup = nodeEngine.toData(output); - atomicReferenceContainer.set(backup); + atomicReferenceContainer.set(serializedOutput); + backup = serializedOutput; } } diff --git a/hazelcast/src/test/java/com/hazelcast/concurrent/atomicreference/AtomicReferenceAbstractTest.java b/hazelcast/src/test/java/com/hazelcast/concurrent/atomicreference/AtomicReferenceAbstractTest.java index d60d16158e81..8ea6d6f1ecf1 100644 --- a/hazelcast/src/test/java/com/hazelcast/concurrent/atomicreference/AtomicReferenceAbstractTest.java +++ b/hazelcast/src/test/java/com/hazelcast/concurrent/atomicreference/AtomicReferenceAbstractTest.java @@ -325,12 +325,36 @@ public void testToString() { public void getAndAlter_when_same_reference() { BitSet bitSet = new BitSet(); IAtomicReference ref2 = newInstance(); + + ref2.set(bitSet); + assertEquals(bitSet, ref2.getAndAlter(new FailingFunctionAlter())); + + bitSet.set(100); + assertEquals(bitSet, ref2.get()); + } + + @Test + public void alterAndGet_when_same_reference() { + BitSet bitSet = new BitSet(); + IAtomicReference ref2 = newInstance(); + ref2.set(bitSet); bitSet.set(100); + assertEquals(bitSet, ref2.alterAndGet(new FailingFunctionAlter())); assertEquals(bitSet, ref2.get()); } + @Test + public void alter_when_same_reference() { + BitSet bitSet = new BitSet(); + IAtomicReference ref2 = newInstance(); + ref2.set(bitSet); + bitSet.set(100); + ref2.alter(new FailingFunctionAlter()); + assertEquals(bitSet, ref2.get()); + } + private static class FailingFunctionAlter implements IFunction { @Override public BitSet apply(BitSet input) {