From 6fd68267cbef8fc14ce10e26e51b85221558bf4f Mon Sep 17 00:00:00 2001 From: ningwei Date: Thu, 18 Oct 2018 10:50:39 +0800 Subject: [PATCH] DATAREDIS-872 - Fix race condition in RedisAtomic counters initialization. We now use setIfAbsent to initialize RedisAtomic counters if no initial value was given. Using setIfAbsent turns the initialization into a single atomic step that prevents race conditions of the previously check and set method that required two Redis commands. Previously, concurrent processes (threads, external changes to Redis) could set the initial value between the existence check and the value set operation that caused the last participant to win. Original pull request: #367. --- .../support/atomic/RedisAtomicDouble.java | 19 +++++++++++++------ .../support/atomic/RedisAtomicInteger.java | 19 +++++++++++++------ .../redis/support/atomic/RedisAtomicLong.java | 19 +++++++++++++------ 3 files changed, 39 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/springframework/data/redis/support/atomic/RedisAtomicDouble.java b/src/main/java/org/springframework/data/redis/support/atomic/RedisAtomicDouble.java index 0bd89c090..cbfe1f1ea 100644 --- a/src/main/java/org/springframework/data/redis/support/atomic/RedisAtomicDouble.java +++ b/src/main/java/org/springframework/data/redis/support/atomic/RedisAtomicDouble.java @@ -40,6 +40,7 @@ import org.springframework.util.Assert; * @author Thomas Darimont * @author Christoph Strobl * @author Mark Paluch + * @author Ning Wei */ public class RedisAtomicDouble extends Number implements Serializable, BoundKeyOperations { @@ -88,9 +89,7 @@ public class RedisAtomicDouble extends Number implements Serializable, BoundKeyO this.operations = generalOps.opsForValue(); if (initialValue == null) { - if (this.operations.get(redisCounter) == null) { - set(0); - } + setIfAbsent(0); } else { set(initialValue); } @@ -134,9 +133,7 @@ public class RedisAtomicDouble extends Number implements Serializable, BoundKeyO this.operations = generalOps.opsForValue(); if (initialValue == null) { - if (this.operations.get(redisCounter) == null) { - set(0); - } + setIfAbsent(0); } else { set(initialValue); } @@ -166,6 +163,16 @@ public class RedisAtomicDouble extends Number implements Serializable, BoundKeyO operations.set(key, newValue); } + /** + * Sets to the given value, only if {@code key} does not exist. + * + * @param newValue the new value. + * @return true if successful. False return indicates that {@code key} already existed. + */ + public Boolean setIfAbsent(double newValue) { + return operations.setIfAbsent(key, newValue); + } + /** * Set to the given value and return the old value. * diff --git a/src/main/java/org/springframework/data/redis/support/atomic/RedisAtomicInteger.java b/src/main/java/org/springframework/data/redis/support/atomic/RedisAtomicInteger.java index e3926aa46..fe48cb933 100644 --- a/src/main/java/org/springframework/data/redis/support/atomic/RedisAtomicInteger.java +++ b/src/main/java/org/springframework/data/redis/support/atomic/RedisAtomicInteger.java @@ -40,6 +40,7 @@ import org.springframework.util.Assert; * @author Thomas Darimont * @author Christoph Strobl * @author Mark Paluch + * @author Ning Wei * @see java.util.concurrent.atomic.AtomicInteger */ public class RedisAtomicInteger extends Number implements Serializable, BoundKeyOperations { @@ -111,9 +112,7 @@ public class RedisAtomicInteger extends Number implements Serializable, BoundKey this.operations = generalOps.opsForValue(); if (initialValue == null) { - if (this.operations.get(redisCounter) == null) { - set(0); - } + setIfAbsent(0); } else { set(initialValue); } @@ -132,9 +131,7 @@ public class RedisAtomicInteger extends Number implements Serializable, BoundKey this.operations = generalOps.opsForValue(); if (initialValue == null) { - if (this.operations.get(redisCounter) == null) { - set(0); - } + setIfAbsent(0); } else { set(initialValue); } @@ -164,6 +161,16 @@ public class RedisAtomicInteger extends Number implements Serializable, BoundKey operations.set(key, newValue); } + /** + * Sets to the given value, only if {@code key} does not exist. + * + * @param newValue the new value. + * @return true if successful. False return indicates that {@code key} already existed. + */ + public Boolean setIfAbsent(int newValue) { + return operations.setIfAbsent(key, newValue); + } + /** * Set to the given value and return the old value. * diff --git a/src/main/java/org/springframework/data/redis/support/atomic/RedisAtomicLong.java b/src/main/java/org/springframework/data/redis/support/atomic/RedisAtomicLong.java index f8433f90f..c17f06f1b 100644 --- a/src/main/java/org/springframework/data/redis/support/atomic/RedisAtomicLong.java +++ b/src/main/java/org/springframework/data/redis/support/atomic/RedisAtomicLong.java @@ -40,6 +40,7 @@ import org.springframework.util.Assert; * @author Thomas Darimont * @author Christoph Strobl * @author Mark Paluch + * @author Ning Wei * @see java.util.concurrent.atomic.AtomicLong */ public class RedisAtomicLong extends Number implements Serializable, BoundKeyOperations { @@ -88,9 +89,7 @@ public class RedisAtomicLong extends Number implements Serializable, BoundKeyOpe this.operations = generalOps.opsForValue(); if (initialValue == null) { - if (this.operations.get(redisCounter) == null) { - set(0); - } + setIfAbsent(0); } else { set(initialValue); } @@ -138,9 +137,7 @@ public class RedisAtomicLong extends Number implements Serializable, BoundKeyOpe this.operations = generalOps.opsForValue(); if (initialValue == null) { - if (this.operations.get(redisCounter) == null) { - set(0); - } + setIfAbsent(0); } else { set(initialValue); } @@ -170,6 +167,16 @@ public class RedisAtomicLong extends Number implements Serializable, BoundKeyOpe operations.set(key, newValue); } + /** + * Sets to the given value, only if {@code key} does not exist. + * + * @param newValue the new value. + * @return true if successful. False return indicates that {@code key} already existed. + */ + public Boolean setIfAbsent(long newValue) { + return operations.setIfAbsent(key, newValue); + } + /** * Set to the given value and return the old value. *