2015-03-01 22:20 GMT+01:00 Oliver Heger <oliver.he...@oliver-heger.de>:
>
>
> Am 01.03.2015 um 17:52 schrieb Benedikt Ritter:
> > 2015-02-26 21:29 GMT+01:00 Oliver Heger <oliver.he...@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.
>
We have several issues here:
1. Find a name for the proposed initializer implementation. How about
BlockingInitizalizer?
2. Fix the bug in AtomicSafeInitializer (hanging threads on exception). My
opinion is, that BusyWaitInitializer would be a much better name for this
class :o)
3. Compare the new implementation and LazyInitializer. Is the
BlockingInitializer a replacement for LazyInitializer?
Benedikt
>
> 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.he...@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 <brit...@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-unsubscr...@commons.apache.org
> >>>> For additional commands, e-mail: dev-h...@commons.apache.org
> >>>>
> >>>>
> >>>
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> >> For additional commands, e-mail: dev-h...@commons.apache.org
> >>
> >>
> >
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
>
--
http://people.apache.org/~britter/
http://www.systemoutprintln.de/
http://twitter.com/BenediktRitter
http://github.com/britter
