From 3f1016c9d7d2fe161b1cac84e922a839e3e6344a 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 | 20 +++++++++++++------ .../redis/support/atomic/RedisAtomicLong.java | 20 +++++++++++++------ 3 files changed, 41 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 d47c75809..47b016927 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 @@ -41,6 +41,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); } @@ -132,9 +131,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); } @@ -164,6 +161,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); + } + /** * Atomically sets to the given value and returns 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 a2e44270f..74c620414 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,8 @@ 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 +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); } @@ -130,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); } @@ -162,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 give 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 2f78d25a1..9b983116b 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 @@ -42,6 +42,8 @@ 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 +90,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 +138,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 +168,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); + } + /** * Atomically sets to the given value and returns the old value. *