From e6c26ad93ed2561c32b4fe3725e8e71809004c44 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 c8218077b..0c08b38e4 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 @@ -39,6 +39,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 { @@ -87,9 +88,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); } @@ -133,9 +132,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); } @@ -165,6 +162,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 1a13c9a5f..0fde5096f 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 @@ -39,6 +39,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 { @@ -110,9 +111,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); } @@ -131,9 +130,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); } @@ -163,6 +160,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 6b3e06ebd..1967d9aa8 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. *