Skip to content

Commit 8b4d112

Browse files
committed
Improve atomicity in DefaultRedisCacheWriter.doLock().
Original pull request spring-projects#518 Closes spring-projects#1686
1 parent 3d2fdf2 commit 8b4d112

File tree

2 files changed

+48
-5
lines changed

2 files changed

+48
-5
lines changed

Diff for: src/main/java/org/springframework/data/redis/cache/DefaultRedisCacheWriter.java

+5-2
Original file line numberDiff line numberDiff line change
@@ -319,12 +319,15 @@ void lock(String name) {
319319
}
320320

321321
@Nullable
322-
private Boolean doLock(String name, Object contextualKey, @Nullable Object contextualValue,
322+
protected Boolean doLock(String name, Object contextualKey, @Nullable Object contextualValue,
323323
RedisConnection connection) {
324324

325325
Expiration expiration = Expiration.from(this.lockTtl.getTimeToLive(contextualKey, contextualValue));
326326

327-
return connection.stringCommands().set(createCacheLockKey(name), new byte[0], expiration, SetOption.SET_IF_ABSENT);
327+
while (!ObjectUtils.nullSafeEquals(connection.stringCommands().set(createCacheLockKey(name), new byte[0], expiration, SetOption.SET_IF_ABSENT),true)) {
328+
checkAndPotentiallyWaitUntilUnlocked(name, connection);
329+
}
330+
return true;
328331
}
329332

330333
/**

Diff for: src/test/java/org/springframework/data/redis/cache/DefaultRedisCacheWriterTests.java

+43-3
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,12 @@
2222
import java.nio.charset.StandardCharsets;
2323
import java.time.Duration;
2424
import java.util.Collection;
25-
import java.util.concurrent.CountDownLatch;
26-
import java.util.concurrent.ExecutionException;
27-
import java.util.concurrent.TimeUnit;
25+
import java.util.concurrent.*;
2826
import java.util.concurrent.atomic.AtomicReference;
2927
import java.util.function.Consumer;
3028

3129
import org.junit.jupiter.api.BeforeEach;
30+
import org.junit.jupiter.api.Disabled;
3231
import org.springframework.data.redis.connection.RedisConnection;
3332
import org.springframework.data.redis.connection.RedisConnectionFactory;
3433
import org.springframework.data.redis.connection.RedisStringCommands.SetOption;
@@ -38,6 +37,7 @@
3837
import org.springframework.data.redis.test.condition.RedisDriver;
3938
import org.springframework.data.redis.test.extension.parametrized.MethodSource;
4039
import org.springframework.data.redis.test.extension.parametrized.ParameterizedRedisTest;
40+
import org.springframework.lang.Nullable;
4141

4242
/**
4343
* Integration tests for {@link DefaultRedisCacheWriter}.
@@ -419,6 +419,46 @@ void noOpStatisticsCollectorReturnsEmptyStatsInstance() {
419419
assertThat(stats.getPuts()).isZero();
420420
}
421421

422+
@ParameterizedRedisTest
423+
@Disabled
424+
void doLockShouldGetLock() throws InterruptedException {
425+
426+
int threadCount = 3;
427+
428+
DefaultRedisCacheWriter cw = new DefaultRedisCacheWriter(connectionFactory, Duration.ofMillis(50),
429+
BatchStrategies.keys()){
430+
@Nullable
431+
protected Boolean doLock(String name, Object contextualKey, @Nullable Object contextualValue,
432+
RedisConnection connection) {
433+
Boolean doLock = super.doLock(name, contextualKey, contextualValue, connection);
434+
assertThat(doLock).isTrue();
435+
return doLock;
436+
}
437+
};
438+
439+
CountDownLatch beforeWrite = new CountDownLatch(threadCount);
440+
CountDownLatch afterWrite = new CountDownLatch(threadCount);
441+
442+
cw.lock(CACHE_NAME);
443+
444+
for (int i = 0; i < threadCount; i++) {
445+
Thread th = new Thread(() -> {
446+
beforeWrite.countDown();
447+
cw.putIfAbsent(CACHE_NAME, binaryCacheKey, binaryCacheValue, Duration.ZERO);
448+
afterWrite.countDown();
449+
});
450+
451+
th.start();
452+
}
453+
454+
beforeWrite.await();
455+
456+
Thread.sleep(200);
457+
458+
cw.unlock(CACHE_NAME);
459+
afterWrite.await();
460+
461+
}
422462
private void doWithConnection(Consumer<RedisConnection> callback) {
423463

424464
try (RedisConnection connection = connectionFactory.getConnection()) {

0 commit comments

Comments
 (0)