Author: britter Date: Thu Feb 26 08:13:58 2015 New Revision: 1662379 URL: http://svn.apache.org/r1662379 Log: Reverting changes from r1661762 (LANG-1086) for now until we have consensus about this change.
Modified: commons/proper/lang/trunk/src/changes/changes.xml commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/concurrent/AtomicSafeInitializer.java commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/AbstractConcurrentInitializerTest.java commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/AtomicInitializerTest.java commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/AtomicSafeInitializerTest.java commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/LazyInitializerTest.java Modified: commons/proper/lang/trunk/src/changes/changes.xml URL: http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/changes/changes.xml?rev=1662379&r1=1662378&r2=1662379&view=diff ============================================================================== --- commons/proper/lang/trunk/src/changes/changes.xml [utf-8] (original) +++ commons/proper/lang/trunk/src/changes/changes.xml [utf-8] Thu Feb 26 08:13:58 2015 @@ -22,7 +22,6 @@ <body> <release version="3.4" date="tba" description="tba"> - <action issue="LANG-1086" type="update" dev="britter">Remove busy wait from AtomicSafeInitializer.get()</action> <action issue="LANG-1081" type="fix" dev="britter" due-to="Jonathan Baker">DiffBuilder.append(String, Object left, Object right) does not do a left.equals(right) check</action> <action issue="LANG-1055" type="fix" dev="britter" due-to="Jonathan Baker">StrSubstitutor.replaceSystemProperties does not work consistently</action> <action issue="LANG-1082" type="add" dev="britter" due-to="Jonathan Baker">Add option to disable the "objectsTriviallyEqual" test in DiffBuilder</action> Modified: commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/concurrent/AtomicSafeInitializer.java URL: http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/concurrent/AtomicSafeInitializer.java?rev=1662379&r1=1662378&r2=1662379&view=diff ============================================================================== --- commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/concurrent/AtomicSafeInitializer.java (original) +++ commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/concurrent/AtomicSafeInitializer.java Thu Feb 26 08:13:58 2015 @@ -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 AtomicSafeInitiali /** Holds the reference to the managed object. */ private final AtomicReference<T> reference = new AtomicReference<T>(); - /** Holds the exception that terminated the initialize() method, if an exception was thrown */ - private final AtomicReference<ConcurrentException> referenceExc = new AtomicReference<ConcurrentException>(); - - /** 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()); } } Modified: commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/AbstractConcurrentInitializerTest.java URL: http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/AbstractConcurrentInitializerTest.java?rev=1662379&r1=1662378&r2=1662379&view=diff ============================================================================== --- commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/AbstractConcurrentInitializerTest.java (original) +++ commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/AbstractConcurrentInitializerTest.java Thu Feb 26 08:13:58 2015 @@ -18,8 +18,6 @@ package org.apache.commons.lang3.concurr 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 AbstractConcurrent @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<Object> initializer = expectExceptions ? - createExceptionThrowingInitializer() : - createInitializer(); - + final ConcurrentInitializer<Object> initializer = createInitializer(); final int threadCount = 20; final CountDownLatch startLatch = new CountDownLatch(1); class GetThread extends Thread { @@ -142,18 +106,9 @@ public abstract class AbstractConcurrent } // 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 AbstractConcurrent * @return the initializer object to be tested */ protected abstract ConcurrentInitializer<Object> createInitializer(); - - /** - * Creates a {@link ConcurrentInitializer} object that always throws - * exceptions. - * - * @return - */ - protected abstract ConcurrentInitializer<Object> createExceptionThrowingInitializer(); } Modified: commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/AtomicInitializerTest.java URL: http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/AtomicInitializerTest.java?rev=1662379&r1=1662378&r2=1662379&view=diff ============================================================================== --- commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/AtomicInitializerTest.java (original) +++ commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/AtomicInitializerTest.java Thu Feb 26 08:13:58 2015 @@ -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 exten } }; } - - @Override - protected ConcurrentInitializer<Object> createExceptionThrowingInitializer() { - return new ExceptionThrowingAtomicSafeInitializerTestImpl(); - } - - /** - * A concrete test implementation of {@code AtomicSafeInitializer}. This - * implementation always throws an exception. - */ - private class ExceptionThrowingAtomicSafeInitializerTestImpl extends AtomicSafeInitializer<Object> { - @Override - protected Object initialize() throws ConcurrentException { - throw new ConcurrentException(testExceptionMessage, testCauseException); - } - } } Modified: commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/AtomicSafeInitializerTest.java URL: http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/AtomicSafeInitializerTest.java?rev=1662379&r1=1662378&r2=1662379&view=diff ============================================================================== --- commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/AtomicSafeInitializerTest.java (original) +++ commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/AtomicSafeInitializerTest.java Thu Feb 26 08:13:58 2015 @@ -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); } /** @@ -60,17 +49,6 @@ public class AtomicSafeInitializerTest e } /** - * Returns the exception-throwing initializer to be tested. - * - * @return the {@code AtomicSafeInitializer} under test when validating - * exception handling - */ - @Override - protected ConcurrentInitializer<Object> createExceptionThrowingInitializer() { - return exceptionThrowingInitializer; - } - - /** * Tests that initialize() is called only once. * * @throws org.apache.commons.lang3.concurrent.ConcurrentException because {@link #testGetConcurrent()} may throw it @@ -84,92 +62,6 @@ public class AtomicSafeInitializerTest e 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 e return new Object(); } } - - /** - * A concrete test implementation of {@code AtomicSafeInitializer}. This - * implementation always throws an exception. - */ - private class ExceptionThrowingAtomicSafeInitializerTestImpl extends AtomicSafeInitializer<Object> { - @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<Object> { - 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<Object> initializer; - private Object result; - private boolean caughtException; - - public InitializerTestThread(AtomicSafeInitializer<Object> 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; - } - } } Modified: commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/LazyInitializerTest.java URL: http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/LazyInitializerTest.java?rev=1662379&r1=1662378&r2=1662379&view=diff ============================================================================== --- commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/LazyInitializerTest.java (original) +++ commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/LazyInitializerTest.java Thu Feb 26 08:13:58 2015 @@ -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 return initializer; } - @Override - protected ConcurrentInitializer<Object> 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 return new Object(); } } - - - /** - * A concrete test implementation of {@code AtomicSafeInitializer}. This - * implementation always throws an exception. - */ - private class ExceptionThrowingLazyInitializerTestImpl extends LazyInitializer<Object> { - @Override - protected Object initialize() throws ConcurrentException { - throw new ConcurrentException(testExceptionMessage, testCauseException); - } - } }