Am 01.03.2015 um 17:52 schrieb Benedikt Ritter:
> 2015-02-26 21:29 GMT+01:00 Oliver Heger <[email protected]>:
>
>> 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 <
>> [email protected]>
>>> 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 <[email protected]>:
>>>>>
>>>>>> 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: [email protected]
>>>> For additional commands, e-mail: [email protected]
>>>>
>>>>
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [email protected]
>> For additional commands, e-mail: [email protected]
>>
>>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
