Skip to content

Commit 2736de9

Browse files
committed
Fix JdbcLockRegistry tests
https://build.spring.io/browse/INT-MASTER-901 The `JdbcLockRegistryDifferentClientTests.testOnlyOneLock()` relies on the `ArrayList.isEmpty()` state to proceed with the logic. But since the `ArrayList.size` property is not `volatile`, there is no guarantee for the proper state in the multi-threaded environment like we have in this test-case. * Replace `ArrayList` in the test with the `LinkedBlockingQueue` which already rely on the `AtomicInteger` for the `size` property * Fix `JdbcLockRegistry` tests to shutdown used `ExecutorService` s to ensure set free threads after test suite execution. **Cherry-pick to 4.3.x**
1 parent 55d92ed commit 2736de9

File tree

2 files changed

+75
-76
lines changed

2 files changed

+75
-76
lines changed

spring-integration-jdbc/src/test/java/org/springframework/integration/jdbc/lock/JdbcLockRegistryDifferentClientTests.java

Lines changed: 53 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2016-2017 the original author or authors.
2+
* Copyright 2016-2018 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -45,6 +45,7 @@
4545
import org.springframework.beans.factory.annotation.Autowired;
4646
import org.springframework.context.ConfigurableApplicationContext;
4747
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
48+
import org.springframework.core.task.SimpleAsyncTaskExecutor;
4849
import org.springframework.test.annotation.DirtiesContext;
4950
import org.springframework.test.context.ContextConfiguration;
5051
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
@@ -97,9 +98,7 @@ public void close() {
9798

9899
@Test
99100
public void testSecondThreadLoses() throws Exception {
100-
101101
for (int i = 0; i < 100; i++) {
102-
103102
final JdbcLockRegistry registry1 = this.registry;
104103
final JdbcLockRegistry registry2 = this.child.getBean(JdbcLockRegistry.class);
105104
final Lock lock1 = registry1.obtain("foo");
@@ -108,41 +107,38 @@ public void testSecondThreadLoses() throws Exception {
108107
final CountDownLatch latch2 = new CountDownLatch(1);
109108
final CountDownLatch latch3 = new CountDownLatch(1);
110109
lock1.lockInterruptibly();
111-
Executors.newSingleThreadExecutor().execute(() -> {
112-
Lock lock2 = registry2.obtain("foo");
113-
try {
114-
latch1.countDown();
115-
lock2.lockInterruptibly();
116-
latch2.await(10, TimeUnit.SECONDS);
117-
locked.set(true);
118-
}
119-
catch (InterruptedException e) {
120-
Thread.currentThread().interrupt();
121-
}
122-
finally {
123-
lock2.unlock();
124-
latch3.countDown();
125-
}
126-
});
110+
new SimpleAsyncTaskExecutor()
111+
.execute(() -> {
112+
Lock lock2 = registry2.obtain("foo");
113+
try {
114+
latch1.countDown();
115+
lock2.lockInterruptibly();
116+
latch2.await(10, TimeUnit.SECONDS);
117+
locked.set(true);
118+
}
119+
catch (InterruptedException e) {
120+
Thread.currentThread().interrupt();
121+
}
122+
finally {
123+
lock2.unlock();
124+
latch3.countDown();
125+
}
126+
});
127127
assertTrue(latch1.await(10, TimeUnit.SECONDS));
128128
assertFalse(locked.get());
129129
lock1.unlock();
130130
latch2.countDown();
131131
assertTrue(latch3.await(10, TimeUnit.SECONDS));
132132
assertTrue(locked.get());
133-
134133
}
135-
136134
}
137135

138136
@Test
139137
public void testBothLock() throws Exception {
140-
141138
for (int i = 0; i < 100; i++) {
142-
143139
final JdbcLockRegistry registry1 = this.registry;
144140
final JdbcLockRegistry registry2 = this.child.getBean(JdbcLockRegistry.class);
145-
final List<String> locked = new ArrayList<String>();
141+
final List<String> locked = new ArrayList<>();
146142
final CountDownLatch latch = new CountDownLatch(2);
147143
ExecutorService pool = Executors.newFixedThreadPool(2);
148144
pool.execute(() -> {
@@ -189,9 +185,8 @@ public void testBothLock() throws Exception {
189185
// eventually they both get the lock and release it
190186
assertTrue(locked.contains("1"));
191187
assertTrue(locked.contains("2"));
192-
188+
pool.shutdownNow();
193189
}
194-
195190
}
196191

197192
@Test
@@ -202,17 +197,16 @@ public void testOnlyOneLock() throws Exception {
202197

203198
private void testOnlyOneLock(String id) throws Exception {
204199
for (int i = 0; i < 100; i++) {
205-
206-
final List<String> locked = new ArrayList<String>();
200+
final BlockingQueue<String> locked = new LinkedBlockingQueue<>();
207201
final CountDownLatch latch = new CountDownLatch(20);
208202
ExecutorService pool = Executors.newFixedThreadPool(6);
209203
ArrayList<Callable<Boolean>> tasks = new ArrayList<Callable<Boolean>>();
204+
final DefaultLockRepository client = (id == null) ?
205+
new DefaultLockRepository(this.dataSource) :
206+
new DefaultLockRepository(this.dataSource, id);
207+
client.afterPropertiesSet();
208+
this.context.getAutowireCapableBeanFactory().autowireBean(client);
210209
for (int j = 0; j < 20; j++) {
211-
final DefaultLockRepository client = (id == null) ?
212-
new DefaultLockRepository(this.dataSource) :
213-
new DefaultLockRepository(this.dataSource, id);
214-
client.afterPropertiesSet();
215-
this.context.getAutowireCapableBeanFactory().autowireBean(client);
216210
Callable<Boolean> task = () -> {
217211
Lock lock = new JdbcLockRegistry(client).obtain("foo");
218212
try {
@@ -240,12 +234,10 @@ private void testOnlyOneLock(String id) throws Exception {
240234
pool.invokeAll(tasks);
241235

242236
assertTrue(latch.await(10, TimeUnit.SECONDS));
243-
// eventually they both get the lock and release it
244237
assertEquals(1, locked.size());
245238
assertTrue(locked.contains("done"));
246-
239+
pool.shutdownNow();
247240
}
248-
249241
}
250242

251243
@Test
@@ -255,35 +247,36 @@ public void testExclusiveAccess() throws Exception {
255247
final DefaultLockRepository client2 = new DefaultLockRepository(dataSource);
256248
client2.afterPropertiesSet();
257249
Lock lock1 = new JdbcLockRegistry(client1).obtain("foo");
258-
final BlockingQueue<Integer> data = new LinkedBlockingQueue<Integer>();
250+
final BlockingQueue<Integer> data = new LinkedBlockingQueue<>();
259251
final CountDownLatch latch1 = new CountDownLatch(1);
260252
lock1.lockInterruptibly();
261-
Executors.newSingleThreadExecutor().execute(() -> {
262-
Lock lock2 = new JdbcLockRegistry(client2).obtain("foo");
263-
try {
264-
latch1.countDown();
265-
StopWatch stopWatch = new StopWatch();
266-
stopWatch.start();
267-
lock2.lockInterruptibly();
268-
stopWatch.stop();
269-
data.add(4);
270-
Thread.sleep(10);
271-
data.add(5);
272-
Thread.sleep(10);
273-
data.add(6);
274-
}
275-
catch (InterruptedException e) {
276-
Thread.currentThread().interrupt();
277-
}
278-
finally {
279-
lock2.unlock();
280-
}
281-
});
253+
new SimpleAsyncTaskExecutor()
254+
.execute(() -> {
255+
Lock lock2 = new JdbcLockRegistry(client2).obtain("foo");
256+
try {
257+
latch1.countDown();
258+
StopWatch stopWatch = new StopWatch();
259+
stopWatch.start();
260+
lock2.lockInterruptibly();
261+
stopWatch.stop();
262+
data.add(4);
263+
Thread.sleep(10);
264+
data.add(5);
265+
Thread.sleep(10);
266+
data.add(6);
267+
}
268+
catch (InterruptedException e) {
269+
Thread.currentThread().interrupt();
270+
}
271+
finally {
272+
lock2.unlock();
273+
}
274+
});
282275
assertTrue(latch1.await(10, TimeUnit.SECONDS));
283276
data.add(1);
284-
Thread.sleep(1000);
277+
Thread.sleep(100);
285278
data.add(2);
286-
Thread.sleep(1000);
279+
Thread.sleep(100);
287280
data.add(3);
288281
lock1.unlock();
289282
for (int i = 0; i < 6; i++) {

spring-integration-jdbc/src/test/java/org/springframework/integration/jdbc/lock/JdbcLockRegistryTests.java

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2016 the original author or authors.
2+
* Copyright 2016-2018 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -27,7 +27,6 @@
2727

2828
import java.util.Map;
2929
import java.util.concurrent.CountDownLatch;
30-
import java.util.concurrent.Executors;
3130
import java.util.concurrent.Future;
3231
import java.util.concurrent.TimeUnit;
3332
import java.util.concurrent.atomic.AtomicBoolean;
@@ -38,20 +37,26 @@
3837
import org.junit.runner.RunWith;
3938

4039
import org.springframework.beans.factory.annotation.Autowired;
40+
import org.springframework.core.task.AsyncTaskExecutor;
41+
import org.springframework.core.task.SimpleAsyncTaskExecutor;
4142
import org.springframework.integration.test.util.TestUtils;
4243
import org.springframework.test.annotation.DirtiesContext;
4344
import org.springframework.test.context.ContextConfiguration;
4445
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
4546

4647
/**
4748
* @author Dave Syer
49+
* @author Artem Bilan
50+
*
4851
* @since 4.3
4952
*/
5053
@ContextConfiguration
5154
@RunWith(SpringJUnit4ClassRunner.class)
52-
@DirtiesContext // close at the end after class
55+
@DirtiesContext
5356
public class JdbcLockRegistryTests {
5457

58+
private final AsyncTaskExecutor taskExecutor = new SimpleAsyncTaskExecutor();
59+
5560
@Autowired
5661
private JdbcLockRegistry registry;
5762

@@ -153,7 +158,7 @@ public void testTwoThreadsSecondFailsToGetLock() throws Exception {
153158
lock1.lockInterruptibly();
154159
final AtomicBoolean locked = new AtomicBoolean();
155160
final CountDownLatch latch = new CountDownLatch(1);
156-
Future<Object> result = Executors.newSingleThreadExecutor().submit(() -> {
161+
Future<Object> result = this.taskExecutor.submit(() -> {
157162
Lock lock2 = JdbcLockRegistryTests.this.registry.obtain("foo");
158163
locked.set(lock2.tryLock(200, TimeUnit.MILLISECONDS));
159164
latch.countDown();
@@ -181,7 +186,7 @@ public void testTwoThreads() throws Exception {
181186
final CountDownLatch latch2 = new CountDownLatch(1);
182187
final CountDownLatch latch3 = new CountDownLatch(1);
183188
lock1.lockInterruptibly();
184-
Executors.newSingleThreadExecutor().execute(() -> {
189+
this.taskExecutor.execute(() -> {
185190
Lock lock2 = JdbcLockRegistryTests.this.registry.obtain("foo");
186191
try {
187192
latch1.countDown();
@@ -217,7 +222,7 @@ public void testTwoThreadsDifferentRegistries() throws Exception {
217222
final CountDownLatch latch2 = new CountDownLatch(1);
218223
final CountDownLatch latch3 = new CountDownLatch(1);
219224
lock1.lockInterruptibly();
220-
Executors.newSingleThreadExecutor().execute(() -> {
225+
this.taskExecutor.execute(() -> {
221226
Lock lock2 = registry2.obtain("foo");
222227
try {
223228
latch1.countDown();
@@ -249,16 +254,17 @@ public void testTwoThreadsWrongOneUnlocks() throws Exception {
249254
lock.lockInterruptibly();
250255
final AtomicBoolean locked = new AtomicBoolean();
251256
final CountDownLatch latch = new CountDownLatch(1);
252-
Future<Object> result = Executors.newSingleThreadExecutor().submit(() -> {
253-
try {
254-
lock.unlock();
255-
}
256-
catch (Exception e) {
257-
latch.countDown();
258-
return e;
259-
}
260-
return null;
261-
});
257+
Future<Object> result =
258+
this.taskExecutor.submit(() -> {
259+
try {
260+
lock.unlock();
261+
}
262+
catch (Exception e) {
263+
latch.countDown();
264+
return e;
265+
}
266+
return null;
267+
});
262268
assertTrue(latch.await(10, TimeUnit.SECONDS));
263269
assertFalse(locked.get());
264270
lock.unlock();

0 commit comments

Comments
 (0)