Skip to content

Commit 3cf7cbf

Browse files
chanyoung1998mp911de
authored andcommitted
Improve atomicity in DefaultRedisCacheWriter.doLock().
Closes #1686 Original pull request: #2879
1 parent 0efb7ba commit 3cf7cbf

File tree

2 files changed

+48
-3
lines changed

2 files changed

+48
-3
lines changed

src/main/java/org/springframework/data/redis/cache/DefaultRedisCacheWriter.java

+7-3
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
* @author Mark Paluch
5858
* @author André Prata
5959
* @author John Blum
60+
* @author ChanYoung Joung
6061
* @since 2.0
6162
*/
6263
class DefaultRedisCacheWriter implements RedisCacheWriter {
@@ -319,12 +320,15 @@ void lock(String name) {
319320
}
320321

321322
@Nullable
322-
private Boolean doLock(String name, Object contextualKey, @Nullable Object contextualValue,
323-
RedisConnection connection) {
323+
protected Boolean doLock(String name, Object contextualKey, @Nullable Object contextualValue,
324+
RedisConnection connection) {
324325

325326
Expiration expiration = Expiration.from(this.lockTtl.getTimeToLive(contextualKey, contextualValue));
326327

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

330334
/**

src/test/java/org/springframework/data/redis/cache/DefaultRedisCacheWriterTests.java

+41
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,14 @@
3838
import org.springframework.data.redis.test.condition.RedisDriver;
3939
import org.springframework.data.redis.test.extension.parametrized.MethodSource;
4040
import org.springframework.data.redis.test.extension.parametrized.ParameterizedRedisTest;
41+
import org.springframework.lang.Nullable;
4142

4243
/**
4344
* Integration tests for {@link DefaultRedisCacheWriter}.
4445
*
4546
* @author Christoph Strobl
4647
* @author Mark Paluch
48+
* @author ChanYoung Joung
4749
*/
4850
@MethodSource("testParams")
4951
public class DefaultRedisCacheWriterTests {
@@ -419,6 +421,45 @@ void noOpStatisticsCollectorReturnsEmptyStatsInstance() {
419421
assertThat(stats.getPuts()).isZero();
420422
}
421423

424+
@ParameterizedRedisTest
425+
void doLockShouldGetLock() throws InterruptedException {
426+
427+
int threadCount = 3;
428+
CountDownLatch beforeWrite = new CountDownLatch(threadCount);
429+
CountDownLatch afterWrite = new CountDownLatch(threadCount);
430+
431+
DefaultRedisCacheWriter cw = new DefaultRedisCacheWriter(connectionFactory, Duration.ofMillis(50),
432+
BatchStrategies.keys()){
433+
@Nullable
434+
protected Boolean doLock(String name, Object contextualKey, @Nullable Object contextualValue,
435+
RedisConnection connection) {
436+
Boolean doLock = super.doLock(name, contextualKey, contextualValue, connection);
437+
assertThat(doLock).isTrue();
438+
return doLock;
439+
}
440+
};
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+
}
462+
422463
private void doWithConnection(Consumer<RedisConnection> callback) {
423464

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

0 commit comments

Comments
 (0)