commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Oliver Heger <oliver.he...@oliver-heger.de>
Subject Re: svn commit: r1661762 - in /commons/proper/lang/trunk/src: changes/ main/java/org/apache/commons/lang3/concurrent/ test/java/org/apache/commons/lang3/concurrent/
Date Sun, 01 Mar 2015 21:20:29 GMT


Am 01.03.2015 um 17:52 schrieb Benedikt Ritter:
> 2015-02-26 21:29 GMT+01:00 Oliver Heger <oliver.heger@oliver-heger.de>:
> 
>> Hi Arthur,
>>
>> I don't have any principle objections against your implementation.
>>
>> I just don't want it as a replacement for the AtomicSafeInitializer
>> class. The latter was designed and announces itself to use only atomic
>> variables for its synchronization and no other types of locks. This
>> premise no longer holds. If the implementation is broken and does not
>> work as expected, we should deprecate and later remove it.
>>
> 
> Just deprecating the class if it has a bug (hanging threads when an
> exception is thrown) doesn't seem right to me.
> 
> How about we apply the patch and make it very clear in the release notes
> and JavaDoc, that it's behavior has been changed?

But then the name of the class is completely wrong. It is a bit like

public class ClassWithoutSynchronization {
    public void doSomething() {
        *synchronized*(this) {
            ...
        }
    }
}

For me the name AtomicSafeInitializer indicates that only atomic
variables are used to ensure thread-safety. Everything else would be
surprising, even if it was mentioned in JavaDoc.

Oliver
> 
> Benedikt
> 
> 
>>
>> What I would be interested in is a comparison between your
>> implementation and LazyInitializer. How do these classes behave in
>> typical scenarios and which one is the preferred option under which
>> conditions?
>>
>> If we add your proposal as an additional initializer implementation, we
>> will also have to update the user's guide and give our users appropriate
>> recommendations when they should use which class.
>>
>> And, we will have to pick a meaningful name for your class.
>>
>> Oliver
>>
>> Am 26.02.2015 um 20:51 schrieb Arthur Naseef:
>>> A few questions coming to mind here:
>>>
>>>    - First off, how do we move this forward?
>>>       - What do we do with the new implementation?
>>>       - What is a good way to address the busy-wait in the original code?
>>>       - What is a good way to address the fact that all threads will
>>>       busy-wait in the original code if the initialize() method throws an
>>>       exception?
>>>       - What is a good way to address the original comments which
>> indicate
>>>       this class will perform better when, in fact, the busy waiting may
>> have a
>>>       significant impact on performance?
>>>    - How can we address the fears?  Honestly, I think the comment is
>> less a
>>>    "fear" than a belief, so how do we prove/disprove?
>>>       - What is this other initializer implementation?
>>>       - What are the differences in operation between the two?
>>>
>>> Oliver - I'm reading a lot of concerns here, but not seeing how you would
>>> like to address those concerns.  Please help me on that front.
>>>
>>> Note, by the way, that the new implementation operates the same as the
>>> original code, to the application using it, except in the case of an
>>> exception thrown on the initialize() call, which is problematic now.
>> That
>>> is, this new implementation guarantees initialize() will only ever be
>>> called one time, and it ensures all callers receive the result of that
>>> initialize() method.
>>>
>>> Art
>>>
>>>
>>>
>>> On Mon, Feb 23, 2015 at 1:47 PM, Oliver Heger <
>> oliver.heger@oliver-heger.de>
>>> wrote:
>>>
>>>>
>>>>
>>>> Am 23.02.2015 um 21:35 schrieb Benedikt Ritter:
>>>>> Oliver Heger has raised concerns about this commit in JIRA [1]:
>>>>>
>>>>>> This is a strong change in the behavior of this class. The main
>> property
>>>>> of atomic initializers was that they are non
>>>>>> blocking. Now a blocking wait has been introduced. When there is
so
>> much
>>>>> contention that the busy wait is
>>>>>> actually a problem, wouln't it then be better to directly use a
>> blocking
>>>>> variant like lazy initializer?
>>>>>
>>>>> I've looked through the JavaDoc of AtomicInitializer once more. It
>> says:
>>>>> "Because {@code AtomicSafeInitializer} does not use synchronization at
>>>> all
>>>>> it probably outruns {@link LazyInitializer}, at least under low or
>>>> moderate
>>>>> concurrent access."
>>>>>
>>>>> This is the only thing I can find regarding concurrency properties of
>>>>> AtomicInitializer. I think this still holds, doesn't it?
>>>>
>>>> No, the CountDownLatch is synchronized.
>>>>
>>>> There are multiple initializer implementations with different properties
>>>> which are suitable for different use cases and scenarios. The atomic
>>>> initializers are useful if you want to avoid blocking calls, but they
>>>> might be an inferior solution under high contention.
>>>>
>>>> My fear is that this commit optimizes the class for a use case which can
>>>> be served better by another initializer implementation which is already
>>>> blocking; and this optimization has negative impact on the original
>>>> intention of AtomicSafeInitializer.
>>>>
>>>> Oliver
>>>>
>>>>>
>>>>> Benedikt
>>>>>
>>>>> [1] https://issues.apache.org/jira/browse/LANG-1086
>>>>>
>>>>> 2015-02-23 21:15 GMT+01:00 <britter@apache.org>:
>>>>>
>>>>>> Author: britter
>>>>>> Date: Mon Feb 23 20:15:49 2015
>>>>>> New Revision: 1661762
>>>>>>
>>>>>> URL: http://svn.apache.org/r1661762
>>>>>> Log:
>>>>>> LANG-1086: Remove busy wait from AtomicSafeInitializer.get(). This
>> also
>>>>>> fixes #46 from github. Thanks to github user artnaseef.
>>>>>>
>>>>>> 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=1661762&r1=1661761&r2=1661762&view=diff
>>>>>>
>>>>>>
>>>>
>> ==============================================================================
>>>>>> --- commons/proper/lang/trunk/src/changes/changes.xml [utf-8]
>> (original)
>>>>>> +++ commons/proper/lang/trunk/src/changes/changes.xml [utf-8] Mon
Feb
>> 23
>>>>>> 20:15:49 2015
>>>>>> @@ -22,6 +22,7 @@
>>>>>>    <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=1661762&r1=1661761&r2=1661762&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
>>>>>> Mon Feb 23 20:15:49 2015
>>>>>> @@ -16,6 +16,7 @@
>>>>>>   */
>>>>>>  package org.apache.commons.lang3.concurrent;
>>>>>>
>>>>>> +import java.util.concurrent.CountDownLatch;
>>>>>>  import java.util.concurrent.atomic.AtomicReference;
>>>>>>
>>>>>>  /**
>>>>>> @@ -62,20 +63,44 @@ 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
>>>>>> +     * exception or the thread is interrupted waiting for another
>>>> thread
>>>>>> to
>>>>>> +     * complete the initialization
>>>>>>       */
>>>>>>      @Override
>>>>>>      public final T get() throws ConcurrentException {
>>>>>>          T result;
>>>>>>
>>>>>> -        while ((result = reference.get()) == null) {
>>>>>> +        if ((result = reference.get()) == null) {
>>>>>>              if (factory.compareAndSet(null, this)) {
>>>>>> -                reference.set(initialize());
>>>>>> +                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);
>>>>>> +                }
>>>>>>              }
>>>>>>          }
>>>>>>
>>>>>>
>>>>>> 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=1661762&r1=1661761&r2=1661762&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
>>>>>> Mon Feb 23 20:15:49 2015
>>>>>> @@ -18,6 +18,8 @@ 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;
>>>>>>
>>>>>> @@ -72,7 +74,41 @@ public abstract class AbstractConcurrent
>>>>>>      @Test
>>>>>>      public void testGetConcurrent() throws ConcurrentException,
>>>>>>              InterruptedException {
>>>>>> -        final ConcurrentInitializer<Object> initializer =
>>>>>> createInitializer();
>>>>>> +
>>>>>> +        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 int threadCount = 20;
>>>>>>          final CountDownLatch startLatch = new CountDownLatch(1);
>>>>>>          class GetThread extends Thread {
>>>>>> @@ -106,9 +142,18 @@ public abstract class AbstractConcurrent
>>>>>>          }
>>>>>>
>>>>>>          // check results
>>>>>> -        final Object managedObject = initializer.get();
>>>>>> -        for (final GetThread t : threads) {
>>>>>> -            assertEquals("Wrong object", managedObject, t.object);
>>>>>> +        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);
>>>>>> +            }
>>>>>>          }
>>>>>>      }
>>>>>>
>>>>>> @@ -119,4 +164,12 @@ 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=1661762&r1=1661761&r2=1661762&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
>>>>>> Mon Feb 23 20:15:49 2015
>>>>>> @@ -16,12 +16,29 @@
>>>>>>   */
>>>>>>  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.
>>>>>>       *
>>>>>> @@ -36,4 +53,20 @@ 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=1661762&r1=1661761&r2=1661762&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
>>>>>> Mon Feb 23 20:15:49 2015
>>>>>> @@ -17,7 +17,11 @@
>>>>>>  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;
>>>>>> @@ -30,12 +34,19 @@ 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);
>>>>>>      }
>>>>>>
>>>>>>      /**
>>>>>> @@ -49,6 +60,17 @@ 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
>>>>>> @@ -62,6 +84,92 @@ 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()
>>>>>> @@ -78,4 +186,90 @@ 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=1661762&r1=1661761&r2=1661762&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
>>>>>> Mon Feb 23 20:15:49 2015
>>>>>> @@ -17,6 +17,7 @@
>>>>>>  package org.apache.commons.lang3.concurrent;
>>>>>>
>>>>>>  import org.junit.Before;
>>>>>> +import org.junit.Test;
>>>>>>
>>>>>>  /**
>>>>>>   * Test class for {@code LazyInitializer}.
>>>>>> @@ -26,10 +27,16 @@ import org.junit.Before;
>>>>>>  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);
>>>>>>      }
>>>>>>
>>>>>>      /**
>>>>>> @@ -43,6 +50,18 @@ 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
>>>>>> @@ -55,4 +74,16 @@ 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);
>>>>>> +        }
>>>>>> +    }
>>>>>>  }
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>>
>>>>
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
> 
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message