You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Benedikt Ritter <br...@apache.org> on 2015/03/02 07:23:00 UTC
[LANG] Handling LANG-1086 (Was: 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/=
2015-03-01 22:20 GMT+01:00 Oliver Heger <ol...@oliver-heger.de>:
>
>
> Am 01.03.2015 um 17:52 schrieb Benedikt Ritter:
> > 2015-02-26 21:29 GMT+01:00 Oliver Heger <ol...@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.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 <br...@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
>
>
--
http://people.apache.org/~britter/
http://www.systemoutprintln.de/
http://twitter.com/BenediktRitter
http://github.com/britter
Re: [LANG] Handling LANG-1086 (Was: 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/=
Posted by Oliver Heger <ol...@oliver-heger.de>.
Am 06.03.2015 um 08:51 schrieb Benedikt Ritter:
> 2015-03-05 22:00 GMT+01:00 Oliver Heger <ol...@oliver-heger.de>:
>
>>
>>
>> Am 03.03.2015 um 08:35 schrieb Benedikt Ritter:
>>> 2015-03-02 21:34 GMT+01:00 Oliver Heger <ol...@oliver-heger.de>:
>>>
>>>>
>>>>
>>>> Am 02.03.2015 um 07:23 schrieb Benedikt Ritter:
>>>>> 2015-03-01 22:20 GMT+01:00 Oliver Heger <oliver.heger@oliver-heger.de
>>> :
>>>>>
>>>>>>
>>>>>>
>>>>>> 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.
>>>>>>
>>>>>
>>>>> 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?
>>>> Who said that the new initializer is a replacement for LazyInitializer?
>>>>
>>>
>>> Nobody, I was asking a question ;-)
>>
>> Okay, so I hope I have answered it.
>>
>> Oliver
>>
>>>
>>>
>>>>
>>>> LazyInitializer is a direct implementation of the double-check idiom
>>>> for an instance field (refer to Bloch's Effective Java). It is pretty
>>>> lean and elegant.
>>>>
>>>> I assume that the new initializer behaves similar to this class: It uses
>>>> fields with volatile semantics in the first level and synchronization if
>>>> a conflict occurs. But it is more complex. So the question is do these
>>>> two classes have different properties and behavior so that it makes
>>>> sense to include both of them?
>>
>
> I currently don't really know how we can evaluate that. Do you have an
> idea? Sounds like we needed some kind of performance test.
>
>
Yes, indeed. However, it will be very difficult to come up with a
reliable and meaningful test scenario.
>From the theory, I expect that the classes behave differently when
certain aspects vary, e.g. concurrent competition (number of threads) or
complexity of the operation performed by the initializer. The goal is to
give recommendations which implementation should be used under which
conditions.
But I don't have a concrete idea either, and my time is currently pretty
limited.
Oliver
>>>>
>>>> Oliver
>>>>
>>>>>
>>>>> 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.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 <br...@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
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> 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
Re: [LANG] Handling LANG-1086 (Was: 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/=
Posted by Benedikt Ritter <br...@apache.org>.
2015-03-05 22:00 GMT+01:00 Oliver Heger <ol...@oliver-heger.de>:
>
>
> Am 03.03.2015 um 08:35 schrieb Benedikt Ritter:
> > 2015-03-02 21:34 GMT+01:00 Oliver Heger <ol...@oliver-heger.de>:
> >
> >>
> >>
> >> Am 02.03.2015 um 07:23 schrieb Benedikt Ritter:
> >>> 2015-03-01 22:20 GMT+01:00 Oliver Heger <oliver.heger@oliver-heger.de
> >:
> >>>
> >>>>
> >>>>
> >>>> 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.
> >>>>
> >>>
> >>> 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?
> >> Who said that the new initializer is a replacement for LazyInitializer?
> >>
> >
> > Nobody, I was asking a question ;-)
>
> Okay, so I hope I have answered it.
>
> Oliver
>
> >
> >
> >>
> >> LazyInitializer is a direct implementation of the double-check idiom
> >> for an instance field (refer to Bloch's Effective Java). It is pretty
> >> lean and elegant.
> >>
> >> I assume that the new initializer behaves similar to this class: It uses
> >> fields with volatile semantics in the first level and synchronization if
> >> a conflict occurs. But it is more complex. So the question is do these
> >> two classes have different properties and behavior so that it makes
> >> sense to include both of them?
>
I currently don't really know how we can evaluate that. Do you have an
idea? Sounds like we needed some kind of performance test.
> >>
> >> Oliver
> >>
> >>>
> >>> 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.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 <br...@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
> >>>>
> >>>>
> >>>
> >>>
> >>
> >> ---------------------------------------------------------------------
> >> 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
>
>
--
http://people.apache.org/~britter/
http://www.systemoutprintln.de/
http://twitter.com/BenediktRitter
http://github.com/britter
Re: [LANG] Handling LANG-1086 (Was: 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/=
Posted by Oliver Heger <ol...@oliver-heger.de>.
Am 03.03.2015 um 08:35 schrieb Benedikt Ritter:
> 2015-03-02 21:34 GMT+01:00 Oliver Heger <ol...@oliver-heger.de>:
>
>>
>>
>> Am 02.03.2015 um 07:23 schrieb Benedikt Ritter:
>>> 2015-03-01 22:20 GMT+01:00 Oliver Heger <ol...@oliver-heger.de>:
>>>
>>>>
>>>>
>>>> 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.
>>>>
>>>
>>> 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?
>> Who said that the new initializer is a replacement for LazyInitializer?
>>
>
> Nobody, I was asking a question ;-)
Okay, so I hope I have answered it.
Oliver
>
>
>>
>> LazyInitializer is a direct implementation of the double-check idiom
>> for an instance field (refer to Bloch's Effective Java). It is pretty
>> lean and elegant.
>>
>> I assume that the new initializer behaves similar to this class: It uses
>> fields with volatile semantics in the first level and synchronization if
>> a conflict occurs. But it is more complex. So the question is do these
>> two classes have different properties and behavior so that it makes
>> sense to include both of them?
>>
>> Oliver
>>
>>>
>>> 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.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 <br...@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
>>>>
>>>>
>>>
>>>
>>
>> ---------------------------------------------------------------------
>> 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
Re: [LANG] Handling LANG-1086 (Was: 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/=
Posted by Benedikt Ritter <br...@apache.org>.
2015-03-02 21:34 GMT+01:00 Oliver Heger <ol...@oliver-heger.de>:
>
>
> Am 02.03.2015 um 07:23 schrieb Benedikt Ritter:
> > 2015-03-01 22:20 GMT+01:00 Oliver Heger <ol...@oliver-heger.de>:
> >
> >>
> >>
> >> 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.
> >>
> >
> > 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?
> Who said that the new initializer is a replacement for LazyInitializer?
>
Nobody, I was asking a question ;-)
>
> LazyInitializer is a direct implementation of the double-check idiom
> for an instance field (refer to Bloch's Effective Java). It is pretty
> lean and elegant.
>
> I assume that the new initializer behaves similar to this class: It uses
> fields with volatile semantics in the first level and synchronization if
> a conflict occurs. But it is more complex. So the question is do these
> two classes have different properties and behavior so that it makes
> sense to include both of them?
>
> Oliver
>
> >
> > 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.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 <br...@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
> >>
> >>
> >
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
--
http://people.apache.org/~britter/
http://www.systemoutprintln.de/
http://twitter.com/BenediktRitter
http://github.com/britter
Re: [LANG] Handling LANG-1086 (Was: 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/=
Posted by Oliver Heger <ol...@oliver-heger.de>.
Am 02.03.2015 um 07:23 schrieb Benedikt Ritter:
> 2015-03-01 22:20 GMT+01:00 Oliver Heger <ol...@oliver-heger.de>:
>
>>
>>
>> Am 01.03.2015 um 17:52 schrieb Benedikt Ritter:
>>> 2015-02-26 21:29 GMT+01:00 Oliver Heger <ol...@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?
Who said that the new initializer is a replacement for LazyInitializer?
LazyInitializer is a direct implementation of the double-check idiom
for an instance field (refer to Bloch's Effective Java). It is pretty
lean and elegant.
I assume that the new initializer behaves similar to this class: It uses
fields with volatile semantics in the first level and synchronization if
a conflict occurs. But it is more complex. So the question is do these
two classes have different properties and behavior so that it makes
sense to include both of them?
Oliver
>
> 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.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 <br...@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
>>
>>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org