Skip to content

Commit

Permalink
Throw ConcurrentModificationException if the ownership of a RedisLock…
Browse files Browse the repository at this point in the history
…/JdbcLock is expired.
  • Loading branch information
EddieChoCho committed Jun 26, 2024
1 parent 3187ae5 commit 5875522
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.springframework.integration.jdbc.lock;

import java.time.Duration;
import java.util.ConcurrentModificationException;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Map.Entry;
Expand Down Expand Up @@ -349,15 +350,14 @@ public void unlock() {
return;
}
else {
throw new IllegalStateException();
// the lock is no longer owned by current process, the exception should be handle and rollback the execution result
throw new ConcurrentModificationException();
}
}
catch (TransientDataAccessException | TransactionTimedOutException | TransactionSystemException e) {
// try again
}
catch (IllegalStateException e) {
throw new IllegalStateException("Lock was released in the store due to expiration. " +
catch (ConcurrentModificationException e) {
throw new ConcurrentModificationException("Lock was released in the store due to expiration. " +
"The integrity of data protected by this lock may have been compromised.");
}
catch (Exception e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.springframework.integration.jdbc.lock;

import java.util.ArrayList;
import java.util.ConcurrentModificationException;
import java.util.List;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.Callable;
Expand Down Expand Up @@ -315,7 +316,7 @@ public void testOutOfDateLockTaken() throws Exception {
});
assertThat(latch.await(10, TimeUnit.SECONDS)).isTrue();
data.add(2);
assertThatThrownBy(lock1::unlock).isInstanceOf(IllegalStateException.class);
assertThatThrownBy(lock1::unlock).isInstanceOf(ConcurrentModificationException.class);
for (int i = 0; i < 2; i++) {
Integer integer = data.poll(10, TimeUnit.SECONDS);
assertThat(integer).isNotNull();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package org.springframework.integration.jdbc.lock;

import java.util.ConcurrentModificationException;
import java.util.Map;
import java.util.Queue;
import java.util.concurrent.CountDownLatch;
Expand Down Expand Up @@ -501,7 +502,7 @@ public void testTryLockWithCustomTtl() throws Exception {
}

@Test
public void testUnlock_lockStatusIsExpired_lockHasBeenAcquiredByAnotherProcess_DataAccessResourceFailureExceptionWillBeThrown() throws Exception {
public void testUnlock_lockStatusIsExpired_lockHasBeenAcquiredByAnotherProcess_ConcurrentModificationExceptionWillBeThrown() throws Exception {
long ttl = 100;
DefaultLockRepository client1 = new DefaultLockRepository(dataSource);
client1.setApplicationContext(this.context);
Expand All @@ -521,14 +522,14 @@ public void testUnlock_lockStatusIsExpired_lockHasBeenAcquiredByAnotherProcess_D
assertThat(lock2.tryLock()).isTrue();
}
finally {
assertThatExceptionOfType(IllegalStateException.class)
assertThatExceptionOfType(ConcurrentModificationException.class)
.isThrownBy(() -> lock1.unlock());
lock2.unlock();
}
}

@Test
public void testUnlock_lockStatusIsExpired_lockDataHasBeenDeleted_IllegalStateExceptionWillBeThrown() throws Exception {
public void testUnlock_lockStatusIsExpired_lockDataHasBeenDeleted_ConcurrentModificationExceptionWillBeThrown() throws Exception {
JdbcLockRegistry registry = new JdbcLockRegistry(client, 100);
Lock lock = registry.obtain("foo");
try {
Expand All @@ -537,7 +538,7 @@ public void testUnlock_lockStatusIsExpired_lockDataHasBeenDeleted_IllegalStateEx
client.deleteExpired();
}
finally {
assertThatExceptionOfType(IllegalStateException.class)
assertThatExceptionOfType(ConcurrentModificationException.class)
.isThrownBy(() -> lock.unlock());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import java.text.SimpleDateFormat;
import java.util.Collections;
import java.util.ConcurrentModificationException;
import java.util.Date;
import java.util.LinkedHashMap;
import java.util.Map;
Expand Down Expand Up @@ -514,12 +515,12 @@ private void removeLockKey() {
return;
}
else if (Boolean.FALSE.equals(unlinkResult)) {
throw new IllegalStateException("Lock was released in the store due to expiration. " +
throw new ConcurrentModificationException("Lock was released in the store due to expiration. " +
"The integrity of data protected by this lock may have been compromised.");
}
}
if (!removeLockKeyInnerDelete()) {
throw new IllegalStateException("Lock was released in the store due to expiration. " +
throw new ConcurrentModificationException("Lock was released in the store due to expiration. " +
"The integrity of data protected by this lock may have been compromised.");
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package org.springframework.integration.redis.util;

import java.util.ConcurrentModificationException;
import java.util.List;
import java.util.Map;
import java.util.Queue;
Expand Down Expand Up @@ -53,7 +54,6 @@
import org.springframework.integration.test.util.TestUtils;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
import static org.assertj.core.api.Assertions.assertThatNoException;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.Mockito.mock;
Expand Down Expand Up @@ -162,7 +162,7 @@ void testTryLockWithCustomTtl(RedisLockType testRedisLockType) throws Interrupte

@ParameterizedTest
@EnumSource(RedisLockType.class)
void testUnlock_lockStatusIsExpired_IllegalStateExceptionWillBeThrown(RedisLockType testRedisLockType) throws InterruptedException {
void testUnlock_lockStatusIsExpired_ConcurrentModificationExceptionWillBeThrown(RedisLockType testRedisLockType) throws InterruptedException {
RedisLockRegistry registry = new RedisLockRegistry(redisConnectionFactory, this.registryKey, 100);
registry.setRedisLockType(testRedisLockType);
Lock lock = registry.obtain("foo");
Expand All @@ -171,7 +171,7 @@ void testUnlock_lockStatusIsExpired_IllegalStateExceptionWillBeThrown(RedisLockT
Thread.sleep(200);
}
finally {
assertThatThrownBy(lock::unlock).isInstanceOf(IllegalStateException.class);
assertThatThrownBy(lock::unlock).isInstanceOf(ConcurrentModificationException.class);
}
registry.destroy();
}
Expand Down Expand Up @@ -459,9 +459,9 @@ void testExceptionOnExpire(RedisLockType testRedisLockType) throws Exception {
Lock lock1 = registry.obtain("foo");
assertThat(lock1.tryLock()).isTrue();
waitForExpire("foo");
assertThatIllegalStateException()
.isThrownBy(lock1::unlock)
.withMessageContaining("Lock was released in the store due to expiration.");
assertThatThrownBy(lock1::unlock)
.isInstanceOf(ConcurrentModificationException.class)
.hasMessageContaining("Lock was released in the store due to expiration.");
registry.destroy();
}

Expand Down

0 comments on commit 5875522

Please sign in to comment.