From 2d4fbc9aa05b9f4ab91226fe1f9ea1b5965520b1 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 3188c3716..7151030b4 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 @@ -42,6 +42,7 @@ import org.springframework.util.Assert; * @author Christoph Strobl * @author Mark Paluch * @author Graham MacMaster + * @author Ning Wei */ public class RedisAtomicDouble extends Number implements Serializable, BoundKeyOperations { @@ -90,9 +91,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); } @@ -136,9 +135,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); } @@ -168,6 +165,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 ced05f04a..97a877626 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 @@ -42,6 +42,7 @@ import org.springframework.util.Assert; * @author Christoph Strobl * @author Mark Paluch * @author Graham MacMaster + * @author Ning Wei * @see java.util.concurrent.atomic.AtomicInteger */ public class RedisAtomicInteger extends Number implements Serializable, BoundKeyOperations { @@ -113,9 +114,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); } @@ -134,9 +133,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); } @@ -166,6 +163,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 7af0cf100..db195cdbe 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 @@ -43,6 +43,7 @@ import org.springframework.util.Assert; * @author Christoph Strobl * @author Mark Paluch * @author Graham MacMaster + * @author Ning Wei * @see java.util.concurrent.atomic.AtomicLong */ public class RedisAtomicLong extends Number implements Serializable, BoundKeyOperations { @@ -91,9 +92,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); } @@ -141,9 +140,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); } @@ -173,6 +170,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. *