diff --git a/src/changes/changes.xml b/src/changes/changes.xml index c08883917..efb78ebee 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -22,7 +22,6 @@ - Remove busy wait from AtomicSafeInitializer.get() DiffBuilder.append(String, Object left, Object right) does not do a left.equals(right) check StrSubstitutor.replaceSystemProperties does not work consistently Add option to disable the "objectsTriviallyEqual" test in DiffBuilder diff --git a/src/main/java/org/apache/commons/lang3/concurrent/AtomicSafeInitializer.java b/src/main/java/org/apache/commons/lang3/concurrent/AtomicSafeInitializer.java index a1186dba2..5bb5096db 100644 --- a/src/main/java/org/apache/commons/lang3/concurrent/AtomicSafeInitializer.java +++ b/src/main/java/org/apache/commons/lang3/concurrent/AtomicSafeInitializer.java @@ -16,7 +16,6 @@ */ package org.apache.commons.lang3.concurrent; -import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicReference; /** @@ -63,44 +62,20 @@ public abstract class AtomicSafeInitializer implements /** Holds the reference to the managed object. */ private final AtomicReference reference = new AtomicReference(); - /** Holds the exception that terminated the initialize() method, if an exception was thrown */ - private final AtomicReference referenceExc = new AtomicReference(); - - /** Latch for those threads waiting for initialization to complete. */ - private final CountDownLatch latch = new CountDownLatch(1); - /** * Get (and initialize, if not initialized yet) the required object * * @return lazily initialized object * @throws ConcurrentException if the initialization of the object causes an - * exception or the thread is interrupted waiting for another thread to - * complete the initialization + * exception */ @Override public final T get() throws ConcurrentException { T result; - if ((result = reference.get()) == null) { + while ((result = reference.get()) == null) { if (factory.compareAndSet(null, this)) { - try { - reference.set(result = initialize()); - } catch ( ConcurrentException exc ) { - referenceExc.set(exc); - throw exc; - } finally { - latch.countDown(); - } - } else { - try { - latch.await(); - if ( referenceExc.get() != null ) { - throw new ConcurrentException(referenceExc.get().getMessage(), referenceExc.get().getCause()); - } - result = reference.get(); - } catch (InterruptedException intExc) { - throw new ConcurrentException("interrupted waiting for initialization to complete", intExc); - } + reference.set(initialize()); } } diff --git a/src/test/java/org/apache/commons/lang3/concurrent/AbstractConcurrentInitializerTest.java b/src/test/java/org/apache/commons/lang3/concurrent/AbstractConcurrentInitializerTest.java index d3fd4c129..6bd8204af 100644 --- a/src/test/java/org/apache/commons/lang3/concurrent/AbstractConcurrentInitializerTest.java +++ b/src/test/java/org/apache/commons/lang3/concurrent/AbstractConcurrentInitializerTest.java @@ -18,8 +18,6 @@ package org.apache.commons.lang3.concurrent; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertTrue; import java.util.concurrent.CountDownLatch; @@ -74,41 +72,7 @@ public abstract class AbstractConcurrentInitializerTest { @Test public void testGetConcurrent() throws ConcurrentException, InterruptedException { - - this.testGetConcurrentOptionallyWithException(false, null, null); - } - - /** - * Tests the handling of exceptions thrown on the initialized when multiple threads execute concurrently. - * Always an exception with the same message and cause should be thrown. - * - * @throws org.apache.commons.lang3.concurrent.ConcurrentException because the object under test may throw it - * @throws java.lang.InterruptedException because the threading API my throw it - */ - public void testGetConcurrentWithException(String expectedMessage, - Exception expectedCause) - throws ConcurrentException, InterruptedException { - - this.testGetConcurrentOptionallyWithException(true, expectedMessage, expectedCause); - } - - /** - * Tests whether get() can be invoked from multiple threads concurrently. Supports the exception-handling case - * and the normal, non-exception case. - * - * Always the same object should be returned, or an exception with the same message and cause should be thrown. - * - * @throws org.apache.commons.lang3.concurrent.ConcurrentException because the object under test may throw it - * @throws java.lang.InterruptedException because the threading API my throw it - */ - protected void testGetConcurrentOptionallyWithException(boolean expectExceptions, String expectedMessage, - Exception expectedCause) - throws ConcurrentException, InterruptedException { - - final ConcurrentInitializer initializer = expectExceptions ? - createExceptionThrowingInitializer() : - createInitializer(); - + final ConcurrentInitializer initializer = createInitializer(); final int threadCount = 20; final CountDownLatch startLatch = new CountDownLatch(1); class GetThread extends Thread { @@ -142,18 +106,9 @@ public abstract class AbstractConcurrentInitializerTest { } // check results - if ( expectExceptions ) { - for (GetThread t : threads) { - assertTrue(t.object instanceof Exception); - Exception exc = (Exception) t.object; - assertEquals(expectedMessage, exc.getMessage()); - assertSame(expectedCause, exc.getCause()); - } - } else { - final Object managedObject = initializer.get(); - for (final GetThread t : threads) { - assertEquals("Wrong object", managedObject, t.object); - } + final Object managedObject = initializer.get(); + for (final GetThread t : threads) { + assertEquals("Wrong object", managedObject, t.object); } } @@ -164,12 +119,4 @@ public abstract class AbstractConcurrentInitializerTest { * @return the initializer object to be tested */ protected abstract ConcurrentInitializer createInitializer(); - - /** - * Creates a {@link ConcurrentInitializer} object that always throws - * exceptions. - * - * @return - */ - protected abstract ConcurrentInitializer createExceptionThrowingInitializer(); } diff --git a/src/test/java/org/apache/commons/lang3/concurrent/AtomicInitializerTest.java b/src/test/java/org/apache/commons/lang3/concurrent/AtomicInitializerTest.java index 9aa83fcd6..7434bb5dc 100644 --- a/src/test/java/org/apache/commons/lang3/concurrent/AtomicInitializerTest.java +++ b/src/test/java/org/apache/commons/lang3/concurrent/AtomicInitializerTest.java @@ -16,29 +16,12 @@ */ package org.apache.commons.lang3.concurrent; -import org.junit.Test; - /** * Test class for {@code AtomicInitializer}. * * @version $Id$ */ public class AtomicInitializerTest extends AbstractConcurrentInitializerTest { - private Exception testCauseException; - private String testExceptionMessage; - - public AtomicInitializerTest() { - testExceptionMessage = "x-test-exception-message-x"; - testCauseException = new Exception(testExceptionMessage); - } - - @Test - public void testGetConcurrentWithException () - throws ConcurrentException, InterruptedException { - - super.testGetConcurrentWithException(testExceptionMessage, testCauseException); - } - /** * Returns the initializer to be tested. * @@ -53,20 +36,4 @@ public class AtomicInitializerTest extends AbstractConcurrentInitializerTest { } }; } - - @Override - protected ConcurrentInitializer createExceptionThrowingInitializer() { - return new ExceptionThrowingAtomicSafeInitializerTestImpl(); - } - - /** - * A concrete test implementation of {@code AtomicSafeInitializer}. This - * implementation always throws an exception. - */ - private class ExceptionThrowingAtomicSafeInitializerTestImpl extends AtomicSafeInitializer { - @Override - protected Object initialize() throws ConcurrentException { - throw new ConcurrentException(testExceptionMessage, testCauseException); - } - } } diff --git a/src/test/java/org/apache/commons/lang3/concurrent/AtomicSafeInitializerTest.java b/src/test/java/org/apache/commons/lang3/concurrent/AtomicSafeInitializerTest.java index 27f36c7e8..bcf1688cb 100644 --- a/src/test/java/org/apache/commons/lang3/concurrent/AtomicSafeInitializerTest.java +++ b/src/test/java/org/apache/commons/lang3/concurrent/AtomicSafeInitializerTest.java @@ -17,11 +17,7 @@ package org.apache.commons.lang3.concurrent; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertTrue; -import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicInteger; import org.junit.Before; @@ -34,19 +30,12 @@ import org.junit.Test; */ public class AtomicSafeInitializerTest extends AbstractConcurrentInitializerTest { - /** The instance to be tested. */ private AtomicSafeInitializerTestImpl initializer; - private ExceptionThrowingAtomicSafeInitializerTestImpl exceptionThrowingInitializer; - private Exception testCauseException; - private String testExceptionMessage; @Before public void setUp() throws Exception { initializer = new AtomicSafeInitializerTestImpl(); - exceptionThrowingInitializer = new ExceptionThrowingAtomicSafeInitializerTestImpl(); - testExceptionMessage = "x-test-exception-message-x"; - testCauseException = new Exception(testExceptionMessage); } /** @@ -59,17 +48,6 @@ public class AtomicSafeInitializerTest extends return initializer; } - /** - * Returns the exception-throwing initializer to be tested. - * - * @return the {@code AtomicSafeInitializer} under test when validating - * exception handling - */ - @Override - protected ConcurrentInitializer createExceptionThrowingInitializer() { - return exceptionThrowingInitializer; - } - /** * Tests that initialize() is called only once. * @@ -84,92 +62,6 @@ public class AtomicSafeInitializerTest extends initializer.initCounter.get()); } - @Test - public void testExceptionOnInitialize() throws ConcurrentException, - InterruptedException { - - testGetConcurrentWithException(testExceptionMessage, testCauseException); - } - - /** - * Validate the handling of an interrupted exception on a thread waiting for another thread to finish calling the - * initialize() method. - * - * @throws Exception - */ - @Test(timeout = 3000) - public void testInterruptedWaitingOnInitialize() throws Exception { - this.execTestWithWaitingOnInitialize(true); - } - - /** - * Test the success path of two threads reaching the initialization point at the same time. - */ - @Test(timeout = 3000) - public void testOneThreadWaitingForAnotherToInitialize () throws Exception { - execTestWithWaitingOnInitialize(false); - } - - - /** - * Execute a test that requires one thread to be waiting on the initialize() method of another thread. This test - * uses latches to guarantee the code path being tested. - * - * @throws Exception - */ - protected void execTestWithWaitingOnInitialize(boolean interruptInd) throws Exception { - final CountDownLatch startLatch = new CountDownLatch(1); - final CountDownLatch finishLatch = new CountDownLatch(1); - final WaitingInitializerTestImpl initializer = new WaitingInitializerTestImpl(startLatch, finishLatch); - - InitializerTestThread execThread1 = new InitializerTestThread(initializer); - InitializerTestThread execThread2 = new InitializerTestThread(initializer); - - // Start the first thread and wait for it to get into the initialize method so we are sure it is the thread - // executing initialize(). - execThread1.start(); - startLatch.await(); - - // Start the second thread and interrupt it to force the InterruptedException. There is no need to make sure - // the thread reaches the await() call before interrupting it. - execThread2.start(); - - if ( interruptInd ) { - // Interrupt the second thread now and wait for it to complete to ensure it reaches the wait inside the - // get() method. - execThread2.interrupt(); - execThread2.join(); - } - - // Signal the completion of the initialize method now. - finishLatch.countDown(); - - // Wait for the initialize() to finish. - execThread1.join(); - - // Wait for thread2 to finish, if it was not already done - if ( ! interruptInd ) { - execThread2.join(); - } - - // - // Validate: thread1 should have the valid result; thread2 should have caught an interrupted exception, if - // interrupted, or should have the same result otherwise. - // - assertFalse(execThread1.isCaughtException()); - assertSame(initializer.getAnswer(), execThread1.getResult()); - - if ( interruptInd ) { - assertTrue(execThread2.isCaughtException()); - Exception exc = (Exception) execThread2.getResult(); - assertTrue(exc.getCause() instanceof InterruptedException); - assertEquals("interrupted waiting for initialization to complete", exc.getMessage()); - } else { - assertFalse(execThread2.isCaughtException()); - assertSame(initializer.getAnswer(), execThread2.getResult()); - } - } - /** * A concrete test implementation of {@code AtomicSafeInitializer}. This * implementation also counts the number of invocations of the initialize() @@ -186,90 +78,4 @@ public class AtomicSafeInitializerTest extends return new Object(); } } - - /** - * A concrete test implementation of {@code AtomicSafeInitializer}. This - * implementation always throws an exception. - */ - private class ExceptionThrowingAtomicSafeInitializerTestImpl extends AtomicSafeInitializer { - @Override - protected Object initialize() throws ConcurrentException { - throw new ConcurrentException(testExceptionMessage, testCauseException); - } - } - - /** - * Initializer that signals it has started and waits to complete until signalled in order to enable a guaranteed - * order-of-operations. This allows the test code to peg one thread to the initialize method for a period of time - * that the test can dictate. - */ - private class WaitingInitializerTestImpl extends AtomicSafeInitializer { - private final CountDownLatch startedLatch; - private final CountDownLatch finishLatch; - private final Object answer = new Object(); - - public WaitingInitializerTestImpl(CountDownLatch startedLatch, CountDownLatch finishLatch) { - this.startedLatch = startedLatch; - this.finishLatch = finishLatch; - } - - @Override - protected Object initialize() throws ConcurrentException { - this.startedLatch.countDown(); - try { - this.finishLatch.await(); - } catch (InterruptedException intExc) { - throw new ConcurrentException(intExc); - } - - return answer; - } - - public Object getAnswer () { - return answer; - } - } - - /** - * Test executor of the initializer get() operation that captures the result. - */ - private class InitializerTestThread extends Thread { - private AtomicSafeInitializer initializer; - private Object result; - private boolean caughtException; - - public InitializerTestThread(AtomicSafeInitializer initializer) { - super("AtomicSafeInitializer test thread"); - this.initializer = initializer; - } - - @Override - public void run() { - try { - this.result = initializer.get(); - } catch ( ConcurrentException concurrentExc ) { - this.caughtException = true; - this.result = concurrentExc; - } - } - - /** - * Resulting object, if the get() method returned successfully, or exception if an exception was thrown. - * - * @return resulting object or exception from the get() method call. - */ - public Object getResult () { - return this.result; - } - - /** - * Determine whether an exception was caught on the get() call. Does not guarantee that the get() method was - * called or completed. - * - * @return true => exception was caught; false => exception was not caught. - */ - public boolean isCaughtException () { - return this.caughtException; - } - } } diff --git a/src/test/java/org/apache/commons/lang3/concurrent/LazyInitializerTest.java b/src/test/java/org/apache/commons/lang3/concurrent/LazyInitializerTest.java index 45ca35d1f..409234ff1 100644 --- a/src/test/java/org/apache/commons/lang3/concurrent/LazyInitializerTest.java +++ b/src/test/java/org/apache/commons/lang3/concurrent/LazyInitializerTest.java @@ -17,7 +17,6 @@ package org.apache.commons.lang3.concurrent; import org.junit.Before; -import org.junit.Test; /** * Test class for {@code LazyInitializer}. @@ -27,16 +26,10 @@ import org.junit.Test; public class LazyInitializerTest extends AbstractConcurrentInitializerTest { /** The initializer to be tested. */ private LazyInitializerTestImpl initializer; - private ExceptionThrowingLazyInitializerTestImpl exceptionThrowingInitializer; - private Exception testCauseException; - private String testExceptionMessage; @Before public void setUp() throws Exception { initializer = new LazyInitializerTestImpl(); - exceptionThrowingInitializer = new ExceptionThrowingLazyInitializerTestImpl(); - testExceptionMessage = "x-test-exception-message-x"; - testCauseException = new Exception(testExceptionMessage); } /** @@ -50,18 +43,6 @@ public class LazyInitializerTest extends AbstractConcurrentInitializerTest { return initializer; } - @Override - protected ConcurrentInitializer createExceptionThrowingInitializer() { - return exceptionThrowingInitializer; - } - - @Test - public void testGetConcurrentWithException () - throws ConcurrentException, InterruptedException { - - super.testGetConcurrentWithException(testExceptionMessage, testCauseException); - } - /** * A test implementation of LazyInitializer. This class creates a plain * Object. As Object does not provide a specific equals() method, it is easy @@ -74,16 +55,4 @@ public class LazyInitializerTest extends AbstractConcurrentInitializerTest { return new Object(); } } - - - /** - * A concrete test implementation of {@code AtomicSafeInitializer}. This - * implementation always throws an exception. - */ - private class ExceptionThrowingLazyInitializerTestImpl extends LazyInitializer { - @Override - protected Object initialize() throws ConcurrentException { - throw new ConcurrentException(testExceptionMessage, testCauseException); - } - } }