Skip to content

Commit 618866f

Browse files
artembilanartembilan
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** # Conflicts: # spring-integration-jdbc/src/test/java/org/springframework/integration/jdbc/lock/JdbcLockRegistryDifferentClientTests.java # spring-integration-jdbc/src/test/java/org/springframework/integration/jdbc/lock/JdbcLockRegistryTests.java
1 parent 21c4932 commit 618866f

File tree

2 files changed

+110
-135
lines changed

2 files changed

+110
-135
lines changed

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

Lines changed: 53 additions & 68 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,45 +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(new Runnable() {
112-
113-
@Override
114-
public void run() {
115-
Lock lock2 = registry2.obtain("foo");
116-
try {
117-
latch1.countDown();
118-
lock2.lockInterruptibly();
119-
latch2.await(10, TimeUnit.SECONDS);
120-
locked.set(true);
121-
}
122-
catch (InterruptedException e) {
123-
Thread.currentThread().interrupt();
124-
}
125-
finally {
126-
lock2.unlock();
127-
latch3.countDown();
128-
}
129-
}
130-
});
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+
});
131127
assertTrue(latch1.await(10, TimeUnit.SECONDS));
132128
assertFalse(locked.get());
133129
lock1.unlock();
134130
latch2.countDown();
135131
assertTrue(latch3.await(10, TimeUnit.SECONDS));
136132
assertTrue(locked.get());
137-
138133
}
139-
140134
}
141135

142136
@Test
143137
public void testBothLock() throws Exception {
144-
145138
for (int i = 0; i < 100; i++) {
146-
147139
final JdbcLockRegistry registry1 = this.registry;
148140
final JdbcLockRegistry registry2 = this.child.getBean(JdbcLockRegistry.class);
149-
final List<String> locked = new ArrayList<String>();
141+
final List<String> locked = new ArrayList<>();
150142
final CountDownLatch latch = new CountDownLatch(2);
151143
ExecutorService pool = Executors.newFixedThreadPool(2);
152144
pool.execute(new Runnable() {
@@ -201,9 +193,8 @@ public void run() {
201193
// eventually they both get the lock and release it
202194
assertTrue(locked.contains("1"));
203195
assertTrue(locked.contains("2"));
204-
196+
pool.shutdownNow();
205197
}
206-
207198
}
208199

209200
@Test
@@ -214,17 +205,16 @@ public void testOnlyOneLock() throws Exception {
214205

215206
private void testOnlyOneLock(String id) throws Exception {
216207
for (int i = 0; i < 100; i++) {
217-
218-
final List<String> locked = new ArrayList<String>();
208+
final BlockingQueue<String> locked = new LinkedBlockingQueue<>();
219209
final CountDownLatch latch = new CountDownLatch(20);
220210
ExecutorService pool = Executors.newFixedThreadPool(6);
221211
ArrayList<Callable<Boolean>> tasks = new ArrayList<Callable<Boolean>>();
212+
final DefaultLockRepository client = (id == null) ?
213+
new DefaultLockRepository(this.dataSource) :
214+
new DefaultLockRepository(this.dataSource, id);
215+
client.afterPropertiesSet();
216+
this.context.getAutowireCapableBeanFactory().autowireBean(client);
222217
for (int j = 0; j < 20; j++) {
223-
final DefaultLockRepository client = (id == null) ?
224-
new DefaultLockRepository(this.dataSource) :
225-
new DefaultLockRepository(this.dataSource, id);
226-
client.afterPropertiesSet();
227-
this.context.getAutowireCapableBeanFactory().autowireBean(client);
228218
Callable<Boolean> task = new Callable<Boolean>() {
229219

230220
@Override
@@ -256,12 +246,10 @@ public Boolean call() {
256246
pool.invokeAll(tasks);
257247

258248
assertTrue(latch.await(10, TimeUnit.SECONDS));
259-
// eventually they both get the lock and release it
260249
assertEquals(1, locked.size());
261250
assertTrue(locked.contains("done"));
262-
251+
pool.shutdownNow();
263252
}
264-
265253
}
266254

267255
@Test
@@ -271,39 +259,36 @@ public void testExclusiveAccess() throws Exception {
271259
final DefaultLockRepository client2 = new DefaultLockRepository(dataSource);
272260
client2.afterPropertiesSet();
273261
Lock lock1 = new JdbcLockRegistry(client1).obtain("foo");
274-
final BlockingQueue<Integer> data = new LinkedBlockingQueue<Integer>();
262+
final BlockingQueue<Integer> data = new LinkedBlockingQueue<>();
275263
final CountDownLatch latch1 = new CountDownLatch(1);
276264
lock1.lockInterruptibly();
277-
Executors.newSingleThreadExecutor().execute(new Runnable() {
278-
279-
@Override
280-
public void run() {
281-
Lock lock2 = new JdbcLockRegistry(client2).obtain("foo");
282-
try {
283-
latch1.countDown();
284-
StopWatch stopWatch = new StopWatch();
285-
stopWatch.start();
286-
lock2.lockInterruptibly();
287-
stopWatch.stop();
288-
data.add(4);
289-
Thread.sleep(10);
290-
data.add(5);
291-
Thread.sleep(10);
292-
data.add(6);
293-
}
294-
catch (InterruptedException e) {
295-
Thread.currentThread().interrupt();
296-
}
297-
finally {
298-
lock2.unlock();
299-
}
300-
}
301-
});
265+
new SimpleAsyncTaskExecutor()
266+
.execute(() -> {
267+
Lock lock2 = new JdbcLockRegistry(client2).obtain("foo");
268+
try {
269+
latch1.countDown();
270+
StopWatch stopWatch = new StopWatch();
271+
stopWatch.start();
272+
lock2.lockInterruptibly();
273+
stopWatch.stop();
274+
data.add(4);
275+
Thread.sleep(10);
276+
data.add(5);
277+
Thread.sleep(10);
278+
data.add(6);
279+
}
280+
catch (InterruptedException e) {
281+
Thread.currentThread().interrupt();
282+
}
283+
finally {
284+
lock2.unlock();
285+
}
286+
});
302287
assertTrue(latch1.await(10, TimeUnit.SECONDS));
303288
data.add(1);
304-
Thread.sleep(1000);
289+
Thread.sleep(100);
305290
data.add(2);
306-
Thread.sleep(1000);
291+
Thread.sleep(100);
307292
data.add(3);
308293
lock1.unlock();
309294
for (int i = 0; i < 6; i++) {

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

Lines changed: 57 additions & 67 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.
@@ -28,7 +28,6 @@
2828
import java.util.Map;
2929
import java.util.concurrent.Callable;
3030
import java.util.concurrent.CountDownLatch;
31-
import java.util.concurrent.Executors;
3231
import java.util.concurrent.Future;
3332
import java.util.concurrent.TimeUnit;
3433
import java.util.concurrent.atomic.AtomicBoolean;
@@ -39,20 +38,26 @@
3938
import org.junit.runner.RunWith;
4039

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

4748
/**
4849
* @author Dave Syer
50+
* @author Artem Bilan
51+
*
4952
* @since 4.3
5053
*/
5154
@ContextConfiguration
5255
@RunWith(SpringJUnit4ClassRunner.class)
53-
@DirtiesContext // close at the end after class
56+
@DirtiesContext
5457
public class JdbcLockRegistryTests {
5558

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

@@ -154,21 +159,17 @@ public void testTwoThreadsSecondFailsToGetLock() throws Exception {
154159
lock1.lockInterruptibly();
155160
final AtomicBoolean locked = new AtomicBoolean();
156161
final CountDownLatch latch = new CountDownLatch(1);
157-
Future<Object> result = Executors.newSingleThreadExecutor().submit(new Callable<Object>() {
158-
159-
@Override
160-
public Object call() throws Exception {
161-
Lock lock2 = JdbcLockRegistryTests.this.registry.obtain("foo");
162-
locked.set(lock2.tryLock(200, TimeUnit.MILLISECONDS));
163-
latch.countDown();
164-
try {
165-
lock2.unlock();
166-
}
167-
catch (Exception e) {
168-
return e;
169-
}
170-
return null;
162+
Future<Object> result = this.taskExecutor.submit(() -> {
163+
Lock lock2 = JdbcLockRegistryTests.this.registry.obtain("foo");
164+
locked.set(lock2.tryLock(200, TimeUnit.MILLISECONDS));
165+
latch.countDown();
166+
try {
167+
lock2.unlock();
171168
}
169+
catch (Exception e) {
170+
return e;
171+
}
172+
return null;
172173
});
173174
assertTrue(latch.await(10, TimeUnit.SECONDS));
174175
assertFalse(locked.get());
@@ -186,24 +187,20 @@ public void testTwoThreads() throws Exception {
186187
final CountDownLatch latch2 = new CountDownLatch(1);
187188
final CountDownLatch latch3 = new CountDownLatch(1);
188189
lock1.lockInterruptibly();
189-
Executors.newSingleThreadExecutor().execute(new Runnable() {
190-
191-
@Override
192-
public void run() {
193-
Lock lock2 = JdbcLockRegistryTests.this.registry.obtain("foo");
194-
try {
195-
latch1.countDown();
196-
lock2.lockInterruptibly();
197-
latch2.await(10, TimeUnit.SECONDS);
198-
locked.set(true);
199-
}
200-
catch (InterruptedException e) {
201-
Thread.currentThread().interrupt();
202-
}
203-
finally {
204-
lock2.unlock();
205-
latch3.countDown();
206-
}
190+
this.taskExecutor.execute(() -> {
191+
Lock lock2 = JdbcLockRegistryTests.this.registry.obtain("foo");
192+
try {
193+
latch1.countDown();
194+
lock2.lockInterruptibly();
195+
latch2.await(10, TimeUnit.SECONDS);
196+
locked.set(true);
197+
}
198+
catch (InterruptedException e) {
199+
Thread.currentThread().interrupt();
200+
}
201+
finally {
202+
lock2.unlock();
203+
latch3.countDown();
207204
}
208205
});
209206
assertTrue(latch1.await(10, TimeUnit.SECONDS));
@@ -226,24 +223,20 @@ public void testTwoThreadsDifferentRegistries() throws Exception {
226223
final CountDownLatch latch2 = new CountDownLatch(1);
227224
final CountDownLatch latch3 = new CountDownLatch(1);
228225
lock1.lockInterruptibly();
229-
Executors.newSingleThreadExecutor().execute(new Runnable() {
230-
231-
@Override
232-
public void run() {
233-
Lock lock2 = registry2.obtain("foo");
234-
try {
235-
latch1.countDown();
236-
lock2.lockInterruptibly();
237-
latch2.await(10, TimeUnit.SECONDS);
238-
locked.set(true);
239-
}
240-
catch (InterruptedException e) {
241-
Thread.currentThread().interrupt();
242-
}
243-
finally {
244-
lock2.unlock();
245-
latch3.countDown();
246-
}
226+
this.taskExecutor.execute(() -> {
227+
Lock lock2 = registry2.obtain("foo");
228+
try {
229+
latch1.countDown();
230+
lock2.lockInterruptibly();
231+
latch2.await(10, TimeUnit.SECONDS);
232+
locked.set(true);
233+
}
234+
catch (InterruptedException e) {
235+
Thread.currentThread().interrupt();
236+
}
237+
finally {
238+
lock2.unlock();
239+
latch3.countDown();
247240
}
248241
});
249242
assertTrue(latch1.await(10, TimeUnit.SECONDS));
@@ -262,20 +255,17 @@ public void testTwoThreadsWrongOneUnlocks() throws Exception {
262255
lock.lockInterruptibly();
263256
final AtomicBoolean locked = new AtomicBoolean();
264257
final CountDownLatch latch = new CountDownLatch(1);
265-
Future<Object> result = Executors.newSingleThreadExecutor().submit(new Callable<Object>() {
266-
267-
@Override
268-
public Object call() throws Exception {
269-
try {
270-
lock.unlock();
271-
}
272-
catch (Exception e) {
273-
latch.countDown();
274-
return e;
275-
}
276-
return null;
277-
}
278-
});
258+
Future<Object> result =
259+
this.taskExecutor.submit(() -> {
260+
try {
261+
lock.unlock();
262+
}
263+
catch (Exception e) {
264+
latch.countDown();
265+
return e;
266+
}
267+
return null;
268+
});
279269
assertTrue(latch.await(10, TimeUnit.SECONDS));
280270
assertFalse(locked.get());
281271
lock.unlock();

0 commit comments

Comments
 (0)