From 2736de99de7ffaf83eef0b3bb104a9b4656902bb Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Mon, 22 Jan 2018 12:52:49 -0500 Subject: [PATCH] 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** --- .../JdbcLockRegistryDifferentClientTests.java | 113 ++++++++---------- .../jdbc/lock/JdbcLockRegistryTests.java | 38 +++--- 2 files changed, 75 insertions(+), 76 deletions(-) diff --git a/spring-integration-jdbc/src/test/java/org/springframework/integration/jdbc/lock/JdbcLockRegistryDifferentClientTests.java b/spring-integration-jdbc/src/test/java/org/springframework/integration/jdbc/lock/JdbcLockRegistryDifferentClientTests.java index 27e30637e2..c0200a2ff5 100644 --- a/spring-integration-jdbc/src/test/java/org/springframework/integration/jdbc/lock/JdbcLockRegistryDifferentClientTests.java +++ b/spring-integration-jdbc/src/test/java/org/springframework/integration/jdbc/lock/JdbcLockRegistryDifferentClientTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2017 the original author or authors. + * Copyright 2016-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -45,6 +45,7 @@ import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.core.task.SimpleAsyncTaskExecutor; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; @@ -97,9 +98,7 @@ public class JdbcLockRegistryDifferentClientTests { @Test public void testSecondThreadLoses() throws Exception { - for (int i = 0; i < 100; i++) { - final JdbcLockRegistry registry1 = this.registry; final JdbcLockRegistry registry2 = this.child.getBean(JdbcLockRegistry.class); final Lock lock1 = registry1.obtain("foo"); @@ -108,41 +107,38 @@ public class JdbcLockRegistryDifferentClientTests { final CountDownLatch latch2 = new CountDownLatch(1); final CountDownLatch latch3 = new CountDownLatch(1); lock1.lockInterruptibly(); - Executors.newSingleThreadExecutor().execute(() -> { - Lock lock2 = registry2.obtain("foo"); - try { - latch1.countDown(); - lock2.lockInterruptibly(); - latch2.await(10, TimeUnit.SECONDS); - locked.set(true); - } - catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } - finally { - lock2.unlock(); - latch3.countDown(); - } - }); + new SimpleAsyncTaskExecutor() + .execute(() -> { + Lock lock2 = registry2.obtain("foo"); + try { + latch1.countDown(); + lock2.lockInterruptibly(); + latch2.await(10, TimeUnit.SECONDS); + locked.set(true); + } + catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + finally { + lock2.unlock(); + latch3.countDown(); + } + }); assertTrue(latch1.await(10, TimeUnit.SECONDS)); assertFalse(locked.get()); lock1.unlock(); latch2.countDown(); assertTrue(latch3.await(10, TimeUnit.SECONDS)); assertTrue(locked.get()); - } - } @Test public void testBothLock() throws Exception { - for (int i = 0; i < 100; i++) { - final JdbcLockRegistry registry1 = this.registry; final JdbcLockRegistry registry2 = this.child.getBean(JdbcLockRegistry.class); - final List locked = new ArrayList(); + final List locked = new ArrayList<>(); final CountDownLatch latch = new CountDownLatch(2); ExecutorService pool = Executors.newFixedThreadPool(2); pool.execute(() -> { @@ -189,9 +185,8 @@ public class JdbcLockRegistryDifferentClientTests { // eventually they both get the lock and release it assertTrue(locked.contains("1")); assertTrue(locked.contains("2")); - + pool.shutdownNow(); } - } @Test @@ -202,17 +197,16 @@ public class JdbcLockRegistryDifferentClientTests { private void testOnlyOneLock(String id) throws Exception { for (int i = 0; i < 100; i++) { - - final List locked = new ArrayList(); + final BlockingQueue locked = new LinkedBlockingQueue<>(); final CountDownLatch latch = new CountDownLatch(20); ExecutorService pool = Executors.newFixedThreadPool(6); ArrayList> tasks = new ArrayList>(); + final DefaultLockRepository client = (id == null) ? + new DefaultLockRepository(this.dataSource) : + new DefaultLockRepository(this.dataSource, id); + client.afterPropertiesSet(); + this.context.getAutowireCapableBeanFactory().autowireBean(client); for (int j = 0; j < 20; j++) { - final DefaultLockRepository client = (id == null) ? - new DefaultLockRepository(this.dataSource) : - new DefaultLockRepository(this.dataSource, id); - client.afterPropertiesSet(); - this.context.getAutowireCapableBeanFactory().autowireBean(client); Callable task = () -> { Lock lock = new JdbcLockRegistry(client).obtain("foo"); try { @@ -240,12 +234,10 @@ public class JdbcLockRegistryDifferentClientTests { pool.invokeAll(tasks); assertTrue(latch.await(10, TimeUnit.SECONDS)); - // eventually they both get the lock and release it assertEquals(1, locked.size()); assertTrue(locked.contains("done")); - + pool.shutdownNow(); } - } @Test @@ -255,35 +247,36 @@ public class JdbcLockRegistryDifferentClientTests { final DefaultLockRepository client2 = new DefaultLockRepository(dataSource); client2.afterPropertiesSet(); Lock lock1 = new JdbcLockRegistry(client1).obtain("foo"); - final BlockingQueue data = new LinkedBlockingQueue(); + final BlockingQueue data = new LinkedBlockingQueue<>(); final CountDownLatch latch1 = new CountDownLatch(1); lock1.lockInterruptibly(); - Executors.newSingleThreadExecutor().execute(() -> { - Lock lock2 = new JdbcLockRegistry(client2).obtain("foo"); - try { - latch1.countDown(); - StopWatch stopWatch = new StopWatch(); - stopWatch.start(); - lock2.lockInterruptibly(); - stopWatch.stop(); - data.add(4); - Thread.sleep(10); - data.add(5); - Thread.sleep(10); - data.add(6); - } - catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } - finally { - lock2.unlock(); - } - }); + new SimpleAsyncTaskExecutor() + .execute(() -> { + Lock lock2 = new JdbcLockRegistry(client2).obtain("foo"); + try { + latch1.countDown(); + StopWatch stopWatch = new StopWatch(); + stopWatch.start(); + lock2.lockInterruptibly(); + stopWatch.stop(); + data.add(4); + Thread.sleep(10); + data.add(5); + Thread.sleep(10); + data.add(6); + } + catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + finally { + lock2.unlock(); + } + }); assertTrue(latch1.await(10, TimeUnit.SECONDS)); data.add(1); - Thread.sleep(1000); + Thread.sleep(100); data.add(2); - Thread.sleep(1000); + Thread.sleep(100); data.add(3); lock1.unlock(); for (int i = 0; i < 6; i++) { diff --git a/spring-integration-jdbc/src/test/java/org/springframework/integration/jdbc/lock/JdbcLockRegistryTests.java b/spring-integration-jdbc/src/test/java/org/springframework/integration/jdbc/lock/JdbcLockRegistryTests.java index 3f6709f2c7..a7f515cae0 100644 --- a/spring-integration-jdbc/src/test/java/org/springframework/integration/jdbc/lock/JdbcLockRegistryTests.java +++ b/spring-integration-jdbc/src/test/java/org/springframework/integration/jdbc/lock/JdbcLockRegistryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2016 the original author or authors. + * Copyright 2016-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -27,7 +27,6 @@ import static org.junit.Assert.assertTrue; import java.util.Map; import java.util.concurrent.CountDownLatch; -import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -38,6 +37,8 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.core.task.AsyncTaskExecutor; +import org.springframework.core.task.SimpleAsyncTaskExecutor; import org.springframework.integration.test.util.TestUtils; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.ContextConfiguration; @@ -45,13 +46,17 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; /** * @author Dave Syer + * @author Artem Bilan + * * @since 4.3 */ @ContextConfiguration @RunWith(SpringJUnit4ClassRunner.class) -@DirtiesContext // close at the end after class +@DirtiesContext public class JdbcLockRegistryTests { + private final AsyncTaskExecutor taskExecutor = new SimpleAsyncTaskExecutor(); + @Autowired private JdbcLockRegistry registry; @@ -153,7 +158,7 @@ public class JdbcLockRegistryTests { lock1.lockInterruptibly(); final AtomicBoolean locked = new AtomicBoolean(); final CountDownLatch latch = new CountDownLatch(1); - Future result = Executors.newSingleThreadExecutor().submit(() -> { + Future result = this.taskExecutor.submit(() -> { Lock lock2 = JdbcLockRegistryTests.this.registry.obtain("foo"); locked.set(lock2.tryLock(200, TimeUnit.MILLISECONDS)); latch.countDown(); @@ -181,7 +186,7 @@ public class JdbcLockRegistryTests { final CountDownLatch latch2 = new CountDownLatch(1); final CountDownLatch latch3 = new CountDownLatch(1); lock1.lockInterruptibly(); - Executors.newSingleThreadExecutor().execute(() -> { + this.taskExecutor.execute(() -> { Lock lock2 = JdbcLockRegistryTests.this.registry.obtain("foo"); try { latch1.countDown(); @@ -217,7 +222,7 @@ public class JdbcLockRegistryTests { final CountDownLatch latch2 = new CountDownLatch(1); final CountDownLatch latch3 = new CountDownLatch(1); lock1.lockInterruptibly(); - Executors.newSingleThreadExecutor().execute(() -> { + this.taskExecutor.execute(() -> { Lock lock2 = registry2.obtain("foo"); try { latch1.countDown(); @@ -249,16 +254,17 @@ public class JdbcLockRegistryTests { lock.lockInterruptibly(); final AtomicBoolean locked = new AtomicBoolean(); final CountDownLatch latch = new CountDownLatch(1); - Future result = Executors.newSingleThreadExecutor().submit(() -> { - try { - lock.unlock(); - } - catch (Exception e) { - latch.countDown(); - return e; - } - return null; - }); + Future result = + this.taskExecutor.submit(() -> { + try { + lock.unlock(); + } + catch (Exception e) { + latch.countDown(); + return e; + } + return null; + }); assertTrue(latch.await(10, TimeUnit.SECONDS)); assertFalse(locked.get()); lock.unlock();