2015-03-02 21:34 GMT+01:00 Oliver Heger <oliver.he...@oliver-heger.de>:
>
>
> Am 02.03.2015 um 07:23 schrieb Benedikt Ritter:
> > 2015-03-01 22:20 GMT+01:00 Oliver Heger <oliver.he...@oliver-heger.de>:
> >
> >>
> >>
> >> Am 01.03.2015 um 17:52 schrieb Benedikt Ritter:
> >>> 2015-02-26 21:29 GMT+01:00 Oliver Heger <oliver.he...@oliver-heger.de
> >:
> >>>
> >>>> Hi Arthur,
> >>>>
> >>>> I don't have any principle objections against your implementation.
> >>>>
> >>>> I just don't want it as a replacement for the AtomicSafeInitializer
> >>>> class. The latter was designed and announces itself to use only atomic
> >>>> variables for its synchronization and no other types of locks. This
> >>>> premise no longer holds. If the implementation is broken and does not
> >>>> work as expected, we should deprecate and later remove it.
> >>>>
> >>>
> >>> Just deprecating the class if it has a bug (hanging threads when an
> >>> exception is thrown) doesn't seem right to me.
> >>>
> >>> How about we apply the patch and make it very clear in the release
> notes
> >>> and JavaDoc, that it's behavior has been changed?
> >>
> >> But then the name of the class is completely wrong. It is a bit like
> >>
> >> public class ClassWithoutSynchronization {
> >> public void doSomething() {
> >> *synchronized*(this) {
> >> ...
> >> }
> >> }
> >> }
> >>
> >> For me the name AtomicSafeInitializer indicates that only atomic
> >> variables are used to ensure thread-safety. Everything else would be
> >> surprising, even if it was mentioned in JavaDoc.
> >>
> >
> > We have several issues here:
> >
> > 1. Find a name for the proposed initializer implementation. How about
> > BlockingInitizalizer?
> > 2. Fix the bug in AtomicSafeInitializer (hanging threads on exception).
> My
> > opinion is, that BusyWaitInitializer would be a much better name for this
> > class :o)
> > 3. Compare the new implementation and LazyInitializer. Is the
> > BlockingInitializer a replacement for LazyInitializer?
> 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.he...@oliver-heger.de>
> >>>>> wrote:
> >>>>>
> >>>>>>
> >>>>>>
> >>>>>> Am 23.02.2015 um 21:35 schrieb Benedikt Ritter:
> >>>>>>> Oliver Heger has raised concerns about this commit in JIRA [1]:
> >>>>>>>
> >>>>>>>> This is a strong change in the behavior of this class. The main
> >>>> property
> >>>>>>> of atomic initializers was that they are non
> >>>>>>>> blocking. Now a blocking wait has been introduced. When there is
> so
> >>>> much
> >>>>>>> contention that the busy wait is
> >>>>>>>> actually a problem, wouln't it then be better to directly use a
> >>>> blocking
> >>>>>>> variant like lazy initializer?
> >>>>>>>
> >>>>>>> I've looked through the JavaDoc of AtomicInitializer once more. It
> >>>> says:
> >>>>>>> "Because {@code AtomicSafeInitializer} does not use synchronization
> >> at
> >>>>>> all
> >>>>>>> it probably outruns {@link LazyInitializer}, at least under low or
> >>>>>> moderate
> >>>>>>> concurrent access."
> >>>>>>>
> >>>>>>> This is the only thing I can find regarding concurrency properties
> of
> >>>>>>> AtomicInitializer. I think this still holds, doesn't it?
> >>>>>>
> >>>>>> No, the CountDownLatch is synchronized.
> >>>>>>
> >>>>>> There are multiple initializer implementations with different
> >> properties
> >>>>>> which are suitable for different use cases and scenarios. The atomic
> >>>>>> initializers are useful if you want to avoid blocking calls, but
> they
> >>>>>> might be an inferior solution under high contention.
> >>>>>>
> >>>>>> My fear is that this commit optimizes the class for a use case which
> >> can
> >>>>>> be served better by another initializer implementation which is
> >> already
> >>>>>> blocking; and this optimization has negative impact on the original
> >>>>>> intention of AtomicSafeInitializer.
> >>>>>>
> >>>>>> Oliver
> >>>>>>
> >>>>>>>
> >>>>>>> Benedikt
> >>>>>>>
> >>>>>>> [1] https://issues.apache.org/jira/browse/LANG-1086
> >>>>>>>
> >>>>>>> 2015-02-23 21:15 GMT+01:00 <brit...@apache.org>:
> >>>>>>>
> >>>>>>>> Author: britter
> >>>>>>>> Date: Mon Feb 23 20:15:49 2015
> >>>>>>>> New Revision: 1661762
> >>>>>>>>
> >>>>>>>> URL: http://svn.apache.org/r1661762
> >>>>>>>> Log:
> >>>>>>>> LANG-1086: Remove busy wait from AtomicSafeInitializer.get(). This
> >>>> also
> >>>>>>>> fixes #46 from github. Thanks to github user artnaseef.
> >>>>>>>>
> >>>>>>>> Modified:
> >>>>>>>> commons/proper/lang/trunk/src/changes/changes.xml
> >>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/concurrent/AtomicSafeInitializer.java
> >>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/AbstractConcurrentInitializerTest.java
> >>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/AtomicInitializerTest.java
> >>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/AtomicSafeInitializerTest.java
> >>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/LazyInitializerTest.java
> >>>>>>>>
> >>>>>>>> Modified: commons/proper/lang/trunk/src/changes/changes.xml
> >>>>>>>> URL:
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/changes/changes.xml?rev=1661762&r1=1661761&r2=1661762&view=diff
> >>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> ==============================================================================
> >>>>>>>> --- commons/proper/lang/trunk/src/changes/changes.xml [utf-8]
> >>>> (original)
> >>>>>>>> +++ commons/proper/lang/trunk/src/changes/changes.xml [utf-8] Mon
> >> Feb
> >>>> 23
> >>>>>>>> 20:15:49 2015
> >>>>>>>> @@ -22,6 +22,7 @@
> >>>>>>>> <body>
> >>>>>>>>
> >>>>>>>> <release version="3.4" date="tba" description="tba">
> >>>>>>>> + <action issue="LANG-1086" type="update" dev="britter">Remove
> >> busy
> >>>>>>>> wait from AtomicSafeInitializer.get()</action>
> >>>>>>>> <action issue="LANG-1081" type="fix" dev="britter"
> >>>> due-to="Jonathan
> >>>>>>>> Baker">DiffBuilder.append(String, Object left, Object right) does
> >> not
> >>>>>> do a
> >>>>>>>> left.equals(right) check</action>
> >>>>>>>> <action issue="LANG-1055" type="fix" dev="britter"
> >>>> due-to="Jonathan
> >>>>>>>> Baker">StrSubstitutor.replaceSystemProperties does not work
> >>>>>>>> consistently</action>
> >>>>>>>> <action issue="LANG-1082" type="add" dev="britter"
> >>>> due-to="Jonathan
> >>>>>>>> Baker">Add option to disable the "objectsTriviallyEqual" test in
> >>>>>>>> DiffBuilder</action>
> >>>>>>>>
> >>>>>>>> Modified:
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/concurrent/AtomicSafeInitializer.java
> >>>>>>>> URL:
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/concurrent/AtomicSafeInitializer.java?rev=1661762&r1=1661761&r2=1661762&view=diff
> >>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> ==============================================================================
> >>>>>>>> ---
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/concurrent/AtomicSafeInitializer.java
> >>>>>>>> (original)
> >>>>>>>> +++
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/concurrent/AtomicSafeInitializer.java
> >>>>>>>> Mon Feb 23 20:15:49 2015
> >>>>>>>> @@ -16,6 +16,7 @@
> >>>>>>>> */
> >>>>>>>> package org.apache.commons.lang3.concurrent;
> >>>>>>>>
> >>>>>>>> +import java.util.concurrent.CountDownLatch;
> >>>>>>>> import java.util.concurrent.atomic.AtomicReference;
> >>>>>>>>
> >>>>>>>> /**
> >>>>>>>> @@ -62,20 +63,44 @@ public abstract class AtomicSafeInitiali
> >>>>>>>> /** Holds the reference to the managed object. */
> >>>>>>>> private final AtomicReference<T> reference = new
> >>>>>> AtomicReference<T>();
> >>>>>>>>
> >>>>>>>> + /** Holds the exception that terminated the initialize()
> >> method,
> >>>> if
> >>>>>>>> an exception was thrown */
> >>>>>>>> + private final AtomicReference<ConcurrentException>
> >> referenceExc =
> >>>>>> new
> >>>>>>>> AtomicReference<ConcurrentException>();
> >>>>>>>> +
> >>>>>>>> + /** Latch for those threads waiting for initialization to
> >>>>>> complete. */
> >>>>>>>> + private final CountDownLatch latch = new CountDownLatch(1);
> >>>>>>>> +
> >>>>>>>> /**
> >>>>>>>> * Get (and initialize, if not initialized yet) the required
> >>>> object
> >>>>>>>> *
> >>>>>>>> * @return lazily initialized object
> >>>>>>>> * @throws ConcurrentException if the initialization of the
> >>>> object
> >>>>>>>> causes an
> >>>>>>>> - * exception
> >>>>>>>> + * exception or the thread is interrupted waiting for another
> >>>>>> thread
> >>>>>>>> to
> >>>>>>>> + * complete the initialization
> >>>>>>>> */
> >>>>>>>> @Override
> >>>>>>>> public final T get() throws ConcurrentException {
> >>>>>>>> T result;
> >>>>>>>>
> >>>>>>>> - while ((result = reference.get()) == null) {
> >>>>>>>> + if ((result = reference.get()) == null) {
> >>>>>>>> if (factory.compareAndSet(null, this)) {
> >>>>>>>> - reference.set(initialize());
> >>>>>>>> + try {
> >>>>>>>> + reference.set(result = initialize());
> >>>>>>>> + } catch ( ConcurrentException exc ) {
> >>>>>>>> + referenceExc.set(exc);
> >>>>>>>> + throw exc;
> >>>>>>>> + } finally {
> >>>>>>>> + latch.countDown();
> >>>>>>>> + }
> >>>>>>>> + } else {
> >>>>>>>> + try {
> >>>>>>>> + latch.await();
> >>>>>>>> + if ( referenceExc.get() != null ) {
> >>>>>>>> + throw new
> >>>>>>>> ConcurrentException(referenceExc.get().getMessage(),
> >>>>>>>> referenceExc.get().getCause());
> >>>>>>>> + }
> >>>>>>>> + result = reference.get();
> >>>>>>>> + } catch (InterruptedException intExc) {
> >>>>>>>> + throw new ConcurrentException("interrupted
> >>>> waiting
> >>>>>>>> for initialization to complete", intExc);
> >>>>>>>> + }
> >>>>>>>> }
> >>>>>>>> }
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Modified:
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/AbstractConcurrentInitializerTest.java
> >>>>>>>> URL:
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/AbstractConcurrentInitializerTest.java?rev=1661762&r1=1661761&r2=1661762&view=diff
> >>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> ==============================================================================
> >>>>>>>> ---
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/AbstractConcurrentInitializerTest.java
> >>>>>>>> (original)
> >>>>>>>> +++
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/AbstractConcurrentInitializerTest.java
> >>>>>>>> Mon Feb 23 20:15:49 2015
> >>>>>>>> @@ -18,6 +18,8 @@ package org.apache.commons.lang3.concurr
> >>>>>>>>
> >>>>>>>> import static org.junit.Assert.assertEquals;
> >>>>>>>> import static org.junit.Assert.assertNotNull;
> >>>>>>>> +import static org.junit.Assert.assertSame;
> >>>>>>>> +import static org.junit.Assert.assertTrue;
> >>>>>>>>
> >>>>>>>> import java.util.concurrent.CountDownLatch;
> >>>>>>>>
> >>>>>>>> @@ -72,7 +74,41 @@ public abstract class AbstractConcurrent
> >>>>>>>> @Test
> >>>>>>>> public void testGetConcurrent() throws ConcurrentException,
> >>>>>>>> InterruptedException {
> >>>>>>>> - final ConcurrentInitializer<Object> initializer =
> >>>>>>>> createInitializer();
> >>>>>>>> +
> >>>>>>>> + this.testGetConcurrentOptionallyWithException(false,
> null,
> >>>>>> null);
> >>>>>>>> + }
> >>>>>>>> +
> >>>>>>>> + /**
> >>>>>>>> + * Tests the handling of exceptions thrown on the initialized
> >>>> when
> >>>>>>>> multiple threads execute concurrently.
> >>>>>>>> + * Always an exception with the same message and cause should
> >> be
> >>>>>>>> thrown.
> >>>>>>>> + *
> >>>>>>>> + * @throws
> >>>> org.apache.commons.lang3.concurrent.ConcurrentException
> >>>>>>>> because the object under test may throw it
> >>>>>>>> + * @throws java.lang.InterruptedException because the
> threading
> >>>> API
> >>>>>>>> my throw it
> >>>>>>>> + */
> >>>>>>>> + public void testGetConcurrentWithException(String
> >>>> expectedMessage,
> >>>>>>>> + Exception
> >>>> expectedCause)
> >>>>>>>> + throws ConcurrentException, InterruptedException {
> >>>>>>>> +
> >>>>>>>> + this.testGetConcurrentOptionallyWithException(true,
> >>>>>>>> expectedMessage, expectedCause);
> >>>>>>>> + }
> >>>>>>>> +
> >>>>>>>> + /**
> >>>>>>>> + * Tests whether get() can be invoked from multiple threads
> >>>>>>>> concurrently. Supports the exception-handling case
> >>>>>>>> + * and the normal, non-exception case.
> >>>>>>>> + *
> >>>>>>>> + * Always the same object should be returned, or an exception
> >>>> with
> >>>>>>>> the same message and cause should be thrown.
> >>>>>>>> + *
> >>>>>>>> + * @throws
> >>>> org.apache.commons.lang3.concurrent.ConcurrentException
> >>>>>>>> because the object under test may throw it
> >>>>>>>> + * @throws java.lang.InterruptedException because the
> threading
> >>>> API
> >>>>>>>> my throw it
> >>>>>>>> + */
> >>>>>>>> + protected void
> testGetConcurrentOptionallyWithException(boolean
> >>>>>>>> expectExceptions, String expectedMessage,
> >>>>>>>> +
> >> Exception
> >>>>>>>> expectedCause)
> >>>>>>>> + throws ConcurrentException, InterruptedException {
> >>>>>>>> +
> >>>>>>>> + final ConcurrentInitializer<Object> initializer =
> >>>>>>>> expectExceptions ?
> >>>>>>>> + createExceptionThrowingInitializer() :
> >>>>>>>> + createInitializer();
> >>>>>>>> +
> >>>>>>>> final int threadCount = 20;
> >>>>>>>> final CountDownLatch startLatch = new CountDownLatch(1);
> >>>>>>>> class GetThread extends Thread {
> >>>>>>>> @@ -106,9 +142,18 @@ public abstract class AbstractConcurrent
> >>>>>>>> }
> >>>>>>>>
> >>>>>>>> // check results
> >>>>>>>> - final Object managedObject = initializer.get();
> >>>>>>>> - for (final GetThread t : threads) {
> >>>>>>>> - assertEquals("Wrong object", managedObject,
> t.object);
> >>>>>>>> + if ( expectExceptions ) {
> >>>>>>>> + for (GetThread t : threads) {
> >>>>>>>> + assertTrue(t.object instanceof Exception);
> >>>>>>>> + Exception exc = (Exception) t.object;
> >>>>>>>> + assertEquals(expectedMessage, exc.getMessage());
> >>>>>>>> + assertSame(expectedCause, exc.getCause());
> >>>>>>>> + }
> >>>>>>>> + } else {
> >>>>>>>> + final Object managedObject = initializer.get();
> >>>>>>>> + for (final GetThread t : threads) {
> >>>>>>>> + assertEquals("Wrong object", managedObject,
> >>>> t.object);
> >>>>>>>> + }
> >>>>>>>> }
> >>>>>>>> }
> >>>>>>>>
> >>>>>>>> @@ -119,4 +164,12 @@ public abstract class AbstractConcurrent
> >>>>>>>> * @return the initializer object to be tested
> >>>>>>>> */
> >>>>>>>> protected abstract ConcurrentInitializer<Object>
> >>>>>> createInitializer();
> >>>>>>>> +
> >>>>>>>> + /**
> >>>>>>>> + * Creates a {@link ConcurrentInitializer} object that always
> >>>>>> throws
> >>>>>>>> + * exceptions.
> >>>>>>>> + *
> >>>>>>>> + * @return
> >>>>>>>> + */
> >>>>>>>> + protected abstract ConcurrentInitializer<Object>
> >>>>>>>> createExceptionThrowingInitializer();
> >>>>>>>> }
> >>>>>>>>
> >>>>>>>> Modified:
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/AtomicInitializerTest.java
> >>>>>>>> URL:
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/AtomicInitializerTest.java?rev=1661762&r1=1661761&r2=1661762&view=diff
> >>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> ==============================================================================
> >>>>>>>> ---
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/AtomicInitializerTest.java
> >>>>>>>> (original)
> >>>>>>>> +++
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/AtomicInitializerTest.java
> >>>>>>>> Mon Feb 23 20:15:49 2015
> >>>>>>>> @@ -16,12 +16,29 @@
> >>>>>>>> */
> >>>>>>>> package org.apache.commons.lang3.concurrent;
> >>>>>>>>
> >>>>>>>> +import org.junit.Test;
> >>>>>>>> +
> >>>>>>>> /**
> >>>>>>>> * Test class for {@code AtomicInitializer}.
> >>>>>>>> *
> >>>>>>>> * @version $Id$
> >>>>>>>> */
> >>>>>>>> public class AtomicInitializerTest extends
> >>>>>>>> AbstractConcurrentInitializerTest {
> >>>>>>>> + private Exception testCauseException;
> >>>>>>>> + private String testExceptionMessage;
> >>>>>>>> +
> >>>>>>>> + public AtomicInitializerTest() {
> >>>>>>>> + testExceptionMessage = "x-test-exception-message-x";
> >>>>>>>> + testCauseException = new Exception(testExceptionMessage);
> >>>>>>>> + }
> >>>>>>>> +
> >>>>>>>> + @Test
> >>>>>>>> + public void testGetConcurrentWithException ()
> >>>>>>>> + throws ConcurrentException, InterruptedException {
> >>>>>>>> +
> >>>>>>>> +
> super.testGetConcurrentWithException(testExceptionMessage,
> >>>>>>>> testCauseException);
> >>>>>>>> + }
> >>>>>>>> +
> >>>>>>>> /**
> >>>>>>>> * Returns the initializer to be tested.
> >>>>>>>> *
> >>>>>>>> @@ -36,4 +53,20 @@ public class AtomicInitializerTest exten
> >>>>>>>> }
> >>>>>>>> };
> >>>>>>>> }
> >>>>>>>> +
> >>>>>>>> + @Override
> >>>>>>>> + protected ConcurrentInitializer<Object>
> >>>>>>>> createExceptionThrowingInitializer() {
> >>>>>>>> + return new
> >> ExceptionThrowingAtomicSafeInitializerTestImpl();
> >>>>>>>> + }
> >>>>>>>> +
> >>>>>>>> + /**
> >>>>>>>> + * A concrete test implementation of {@code
> >>>> AtomicSafeInitializer}.
> >>>>>>>> This
> >>>>>>>> + * implementation always throws an exception.
> >>>>>>>> + */
> >>>>>>>> + private class ExceptionThrowingAtomicSafeInitializerTestImpl
> >>>>>> extends
> >>>>>>>> AtomicSafeInitializer<Object> {
> >>>>>>>> + @Override
> >>>>>>>> + protected Object initialize() throws ConcurrentException
> {
> >>>>>>>> + throw new ConcurrentException(testExceptionMessage,
> >>>>>>>> testCauseException);
> >>>>>>>> + }
> >>>>>>>> + }
> >>>>>>>> }
> >>>>>>>>
> >>>>>>>> Modified:
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/AtomicSafeInitializerTest.java
> >>>>>>>> URL:
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/AtomicSafeInitializerTest.java?rev=1661762&r1=1661761&r2=1661762&view=diff
> >>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> ==============================================================================
> >>>>>>>> ---
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/AtomicSafeInitializerTest.java
> >>>>>>>> (original)
> >>>>>>>> +++
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/AtomicSafeInitializerTest.java
> >>>>>>>> Mon Feb 23 20:15:49 2015
> >>>>>>>> @@ -17,7 +17,11 @@
> >>>>>>>> package org.apache.commons.lang3.concurrent;
> >>>>>>>>
> >>>>>>>> import static org.junit.Assert.assertEquals;
> >>>>>>>> +import static org.junit.Assert.assertFalse;
> >>>>>>>> +import static org.junit.Assert.assertSame;
> >>>>>>>> +import static org.junit.Assert.assertTrue;
> >>>>>>>>
> >>>>>>>> +import java.util.concurrent.CountDownLatch;
> >>>>>>>> import java.util.concurrent.atomic.AtomicInteger;
> >>>>>>>>
> >>>>>>>> import org.junit.Before;
> >>>>>>>> @@ -30,12 +34,19 @@ import org.junit.Test;
> >>>>>>>> */
> >>>>>>>> public class AtomicSafeInitializerTest extends
> >>>>>>>> AbstractConcurrentInitializerTest {
> >>>>>>>> +
> >>>>>>>> /** The instance to be tested. */
> >>>>>>>> private AtomicSafeInitializerTestImpl initializer;
> >>>>>>>> + private ExceptionThrowingAtomicSafeInitializerTestImpl
> >>>>>>>> exceptionThrowingInitializer;
> >>>>>>>> + private Exception testCauseException;
> >>>>>>>> + private String testExceptionMessage;
> >>>>>>>>
> >>>>>>>> @Before
> >>>>>>>> public void setUp() throws Exception {
> >>>>>>>> initializer = new AtomicSafeInitializerTestImpl();
> >>>>>>>> + exceptionThrowingInitializer = new
> >>>>>>>> ExceptionThrowingAtomicSafeInitializerTestImpl();
> >>>>>>>> + testExceptionMessage = "x-test-exception-message-x";
> >>>>>>>> + testCauseException = new Exception(testExceptionMessage);
> >>>>>>>> }
> >>>>>>>>
> >>>>>>>> /**
> >>>>>>>> @@ -49,6 +60,17 @@ public class AtomicSafeInitializerTest e
> >>>>>>>> }
> >>>>>>>>
> >>>>>>>> /**
> >>>>>>>> + * Returns the exception-throwing initializer to be tested.
> >>>>>>>> + *
> >>>>>>>> + * @return the {@code AtomicSafeInitializer} under test when
> >>>>>>>> validating
> >>>>>>>> + * exception handling
> >>>>>>>> + */
> >>>>>>>> + @Override
> >>>>>>>> + protected ConcurrentInitializer<Object>
> >>>>>>>> createExceptionThrowingInitializer() {
> >>>>>>>> + return exceptionThrowingInitializer;
> >>>>>>>> + }
> >>>>>>>> +
> >>>>>>>> + /**
> >>>>>>>> * Tests that initialize() is called only once.
> >>>>>>>> *
> >>>>>>>> * @throws
> >>>> org.apache.commons.lang3.concurrent.ConcurrentException
> >>>>>>>> because {@link #testGetConcurrent()} may throw it
> >>>>>>>> @@ -62,6 +84,92 @@ public class AtomicSafeInitializerTest e
> >>>>>>>> initializer.initCounter.get());
> >>>>>>>> }
> >>>>>>>>
> >>>>>>>> + @Test
> >>>>>>>> + public void testExceptionOnInitialize() throws
> >>>> ConcurrentException,
> >>>>>>>> + InterruptedException {
> >>>>>>>> +
> >>>>>>>> + testGetConcurrentWithException(testExceptionMessage,
> >>>>>>>> testCauseException);
> >>>>>>>> + }
> >>>>>>>> +
> >>>>>>>> + /**
> >>>>>>>> + * Validate the handling of an interrupted exception on a
> >> thread
> >>>>>>>> waiting for another thread to finish calling the
> >>>>>>>> + * initialize() method.
> >>>>>>>> + *
> >>>>>>>> + * @throws Exception
> >>>>>>>> + */
> >>>>>>>> + @Test(timeout = 3000)
> >>>>>>>> + public void testInterruptedWaitingOnInitialize() throws
> >>>> Exception {
> >>>>>>>> + this.execTestWithWaitingOnInitialize(true);
> >>>>>>>> + }
> >>>>>>>> +
> >>>>>>>> + /**
> >>>>>>>> + * Test the success path of two threads reaching the
> >>>> initialization
> >>>>>>>> point at the same time.
> >>>>>>>> + */
> >>>>>>>> + @Test(timeout = 3000)
> >>>>>>>> + public void testOneThreadWaitingForAnotherToInitialize ()
> >> throws
> >>>>>>>> Exception {
> >>>>>>>> + execTestWithWaitingOnInitialize(false);
> >>>>>>>> + }
> >>>>>>>> +
> >>>>>>>> +
> >>>>>>>> + /**
> >>>>>>>> + * Execute a test that requires one thread to be waiting on
> the
> >>>>>>>> initialize() method of another thread. This test
> >>>>>>>> + * uses latches to guarantee the code path being tested.
> >>>>>>>> + *
> >>>>>>>> + * @throws Exception
> >>>>>>>> + */
> >>>>>>>> + protected void execTestWithWaitingOnInitialize(boolean
> >>>>>> interruptInd)
> >>>>>>>> throws Exception {
> >>>>>>>> + final CountDownLatch startLatch = new CountDownLatch(1);
> >>>>>>>> + final CountDownLatch finishLatch = new CountDownLatch(1);
> >>>>>>>> + final WaitingInitializerTestImpl initializer = new
> >>>>>>>> WaitingInitializerTestImpl(startLatch, finishLatch);
> >>>>>>>> +
> >>>>>>>> + InitializerTestThread execThread1 = new
> >>>>>>>> InitializerTestThread(initializer);
> >>>>>>>> + InitializerTestThread execThread2 = new
> >>>>>>>> InitializerTestThread(initializer);
> >>>>>>>> +
> >>>>>>>> + // Start the first thread and wait for it to get into the
> >>>>>>>> initialize method so we are sure it is the thread
> >>>>>>>> + // executing initialize().
> >>>>>>>> + execThread1.start();
> >>>>>>>> + startLatch.await();
> >>>>>>>> +
> >>>>>>>> + // Start the second thread and interrupt it to force the
> >>>>>>>> InterruptedException. There is no need to make sure
> >>>>>>>> + // the thread reaches the await() call before
> interrupting
> >>>> it.
> >>>>>>>> + execThread2.start();
> >>>>>>>> +
> >>>>>>>> + if ( interruptInd ) {
> >>>>>>>> + // Interrupt the second thread now and wait for it to
> >>>>>>>> complete to ensure it reaches the wait inside the
> >>>>>>>> + // get() method.
> >>>>>>>> + execThread2.interrupt();
> >>>>>>>> + execThread2.join();
> >>>>>>>> + }
> >>>>>>>> +
> >>>>>>>> + // Signal the completion of the initialize method now.
> >>>>>>>> + finishLatch.countDown();
> >>>>>>>> +
> >>>>>>>> + // Wait for the initialize() to finish.
> >>>>>>>> + execThread1.join();
> >>>>>>>> +
> >>>>>>>> + // Wait for thread2 to finish, if it was not already done
> >>>>>>>> + if ( ! interruptInd ) {
> >>>>>>>> + execThread2.join();
> >>>>>>>> + }
> >>>>>>>> +
> >>>>>>>> + //
> >>>>>>>> + // Validate: thread1 should have the valid result;
> thread2
> >>>>>> should
> >>>>>>>> have caught an interrupted exception, if
> >>>>>>>> + // interrupted, or should have the same result
> otherwise.
> >>>>>>>> + //
> >>>>>>>> + assertFalse(execThread1.isCaughtException());
> >>>>>>>> + assertSame(initializer.getAnswer(),
> >> execThread1.getResult());
> >>>>>>>> +
> >>>>>>>> + if ( interruptInd ) {
> >>>>>>>> + assertTrue(execThread2.isCaughtException());
> >>>>>>>> + Exception exc = (Exception) execThread2.getResult();
> >>>>>>>> + assertTrue(exc.getCause() instanceof
> >>>> InterruptedException);
> >>>>>>>> + assertEquals("interrupted waiting for initialization
> to
> >>>>>>>> complete", exc.getMessage());
> >>>>>>>> + } else {
> >>>>>>>> + assertFalse(execThread2.isCaughtException());
> >>>>>>>> + assertSame(initializer.getAnswer(),
> >>>>>> execThread2.getResult());
> >>>>>>>> + }
> >>>>>>>> + }
> >>>>>>>> +
> >>>>>>>> /**
> >>>>>>>> * A concrete test implementation of {@code
> >>>> AtomicSafeInitializer}.
> >>>>>>>> This
> >>>>>>>> * implementation also counts the number of invocations of
> the
> >>>>>>>> initialize()
> >>>>>>>> @@ -78,4 +186,90 @@ public class AtomicSafeInitializerTest e
> >>>>>>>> return new Object();
> >>>>>>>> }
> >>>>>>>> }
> >>>>>>>> +
> >>>>>>>> + /**
> >>>>>>>> + * A concrete test implementation of {@code
> >>>> AtomicSafeInitializer}.
> >>>>>>>> This
> >>>>>>>> + * implementation always throws an exception.
> >>>>>>>> + */
> >>>>>>>> + private class ExceptionThrowingAtomicSafeInitializerTestImpl
> >>>>>> extends
> >>>>>>>> AtomicSafeInitializer<Object> {
> >>>>>>>> + @Override
> >>>>>>>> + protected Object initialize() throws ConcurrentException
> {
> >>>>>>>> + throw new ConcurrentException(testExceptionMessage,
> >>>>>>>> testCauseException);
> >>>>>>>> + }
> >>>>>>>> + }
> >>>>>>>> +
> >>>>>>>> + /**
> >>>>>>>> + * Initializer that signals it has started and waits to
> >> complete
> >>>>>>>> until signalled in order to enable a guaranteed
> >>>>>>>> + * order-of-operations. This allows the test code to peg one
> >>>>>> thread
> >>>>>>>> to the initialize method for a period of time
> >>>>>>>> + * that the test can dictate.
> >>>>>>>> + */
> >>>>>>>> + private class WaitingInitializerTestImpl extends
> >>>>>>>> AtomicSafeInitializer<Object> {
> >>>>>>>> + private final CountDownLatch startedLatch;
> >>>>>>>> + private final CountDownLatch finishLatch;
> >>>>>>>> + private final Object answer = new Object();
> >>>>>>>> +
> >>>>>>>> + public WaitingInitializerTestImpl(CountDownLatch
> >>>> startedLatch,
> >>>>>>>> CountDownLatch finishLatch) {
> >>>>>>>> + this.startedLatch = startedLatch;
> >>>>>>>> + this.finishLatch = finishLatch;
> >>>>>>>> + }
> >>>>>>>> +
> >>>>>>>> + @Override
> >>>>>>>> + protected Object initialize() throws ConcurrentException
> {
> >>>>>>>> + this.startedLatch.countDown();
> >>>>>>>> + try {
> >>>>>>>> + this.finishLatch.await();
> >>>>>>>> + } catch (InterruptedException intExc) {
> >>>>>>>> + throw new ConcurrentException(intExc);
> >>>>>>>> + }
> >>>>>>>> +
> >>>>>>>> + return answer;
> >>>>>>>> + }
> >>>>>>>> +
> >>>>>>>> + public Object getAnswer () {
> >>>>>>>> + return answer;
> >>>>>>>> + }
> >>>>>>>> + }
> >>>>>>>> +
> >>>>>>>> + /**
> >>>>>>>> + * Test executor of the initializer get() operation that
> >> captures
> >>>>>> the
> >>>>>>>> result.
> >>>>>>>> + */
> >>>>>>>> + private class InitializerTestThread extends Thread {
> >>>>>>>> + private AtomicSafeInitializer<Object> initializer;
> >>>>>>>> + private Object result;
> >>>>>>>> + private boolean caughtException;
> >>>>>>>> +
> >>>>>>>> + public
> InitializerTestThread(AtomicSafeInitializer<Object>
> >>>>>>>> initializer) {
> >>>>>>>> + super("AtomicSafeInitializer test thread");
> >>>>>>>> + this.initializer = initializer;
> >>>>>>>> + }
> >>>>>>>> +
> >>>>>>>> + @Override
> >>>>>>>> + public void run() {
> >>>>>>>> + try {
> >>>>>>>> + this.result = initializer.get();
> >>>>>>>> + } catch ( ConcurrentException concurrentExc ) {
> >>>>>>>> + this.caughtException = true;
> >>>>>>>> + this.result = concurrentExc;
> >>>>>>>> + }
> >>>>>>>> + }
> >>>>>>>> +
> >>>>>>>> + /**
> >>>>>>>> + * Resulting object, if the get() method returned
> >>>> successfully,
> >>>>>>>> or exception if an exception was thrown.
> >>>>>>>> + *
> >>>>>>>> + * @return resulting object or exception from the get()
> >>>> method
> >>>>>>>> call.
> >>>>>>>> + */
> >>>>>>>> + public Object getResult () {
> >>>>>>>> + return this.result;
> >>>>>>>> + }
> >>>>>>>> +
> >>>>>>>> + /**
> >>>>>>>> + * Determine whether an exception was caught on the get()
> >>>> call.
> >>>>>>>> Does not guarantee that the get() method was
> >>>>>>>> + * called or completed.
> >>>>>>>> + *
> >>>>>>>> + * @return true => exception was caught; false =>
> exception
> >>>> was
> >>>>>>>> not caught.
> >>>>>>>> + */
> >>>>>>>> + public boolean isCaughtException () {
> >>>>>>>> + return this.caughtException;
> >>>>>>>> + }
> >>>>>>>> + }
> >>>>>>>> }
> >>>>>>>>
> >>>>>>>> Modified:
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/LazyInitializerTest.java
> >>>>>>>> URL:
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/LazyInitializerTest.java?rev=1661762&r1=1661761&r2=1661762&view=diff
> >>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> ==============================================================================
> >>>>>>>> ---
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/LazyInitializerTest.java
> >>>>>>>> (original)
> >>>>>>>> +++
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/concurrent/LazyInitializerTest.java
> >>>>>>>> Mon Feb 23 20:15:49 2015
> >>>>>>>> @@ -17,6 +17,7 @@
> >>>>>>>> package org.apache.commons.lang3.concurrent;
> >>>>>>>>
> >>>>>>>> import org.junit.Before;
> >>>>>>>> +import org.junit.Test;
> >>>>>>>>
> >>>>>>>> /**
> >>>>>>>> * Test class for {@code LazyInitializer}.
> >>>>>>>> @@ -26,10 +27,16 @@ import org.junit.Before;
> >>>>>>>> public class LazyInitializerTest extends
> >>>>>>>> AbstractConcurrentInitializerTest {
> >>>>>>>> /** The initializer to be tested. */
> >>>>>>>> private LazyInitializerTestImpl initializer;
> >>>>>>>> + private ExceptionThrowingLazyInitializerTestImpl
> >>>>>>>> exceptionThrowingInitializer;
> >>>>>>>> + private Exception testCauseException;
> >>>>>>>> + private String testExceptionMessage;
> >>>>>>>>
> >>>>>>>> @Before
> >>>>>>>> public void setUp() throws Exception {
> >>>>>>>> initializer = new LazyInitializerTestImpl();
> >>>>>>>> + exceptionThrowingInitializer = new
> >>>>>>>> ExceptionThrowingLazyInitializerTestImpl();
> >>>>>>>> + testExceptionMessage = "x-test-exception-message-x";
> >>>>>>>> + testCauseException = new Exception(testExceptionMessage);
> >>>>>>>> }
> >>>>>>>>
> >>>>>>>> /**
> >>>>>>>> @@ -43,6 +50,18 @@ public class LazyInitializerTest extends
> >>>>>>>> return initializer;
> >>>>>>>> }
> >>>>>>>>
> >>>>>>>> + @Override
> >>>>>>>> + protected ConcurrentInitializer<Object>
> >>>>>>>> createExceptionThrowingInitializer() {
> >>>>>>>> + return exceptionThrowingInitializer;
> >>>>>>>> + }
> >>>>>>>> +
> >>>>>>>> + @Test
> >>>>>>>> + public void testGetConcurrentWithException ()
> >>>>>>>> + throws ConcurrentException, InterruptedException {
> >>>>>>>> +
> >>>>>>>> +
> super.testGetConcurrentWithException(testExceptionMessage,
> >>>>>>>> testCauseException);
> >>>>>>>> + }
> >>>>>>>> +
> >>>>>>>> /**
> >>>>>>>> * A test implementation of LazyInitializer. This class
> >> creates a
> >>>>>>>> plain
> >>>>>>>> * Object. As Object does not provide a specific equals()
> >> method,
> >>>>>> it
> >>>>>>>> is easy
> >>>>>>>> @@ -55,4 +74,16 @@ public class LazyInitializerTest extends
> >>>>>>>> return new Object();
> >>>>>>>> }
> >>>>>>>> }
> >>>>>>>> +
> >>>>>>>> +
> >>>>>>>> + /**
> >>>>>>>> + * A concrete test implementation of {@code
> >>>> AtomicSafeInitializer}.
> >>>>>>>> This
> >>>>>>>> + * implementation always throws an exception.
> >>>>>>>> + */
> >>>>>>>> + private class ExceptionThrowingLazyInitializerTestImpl
> extends
> >>>>>>>> LazyInitializer<Object> {
> >>>>>>>> + @Override
> >>>>>>>> + protected Object initialize() throws ConcurrentException
> {
> >>>>>>>> + throw new ConcurrentException(testExceptionMessage,
> >>>>>>>> testCauseException);
> >>>>>>>> + }
> >>>>>>>> + }
> >>>>>>>> }
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>>
> ---------------------------------------------------------------------
> >>>>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> >>>>>> For additional commands, e-mail: dev-h...@commons.apache.org
> >>>>>>
> >>>>>>
> >>>>>
> >>>>
> >>>> ---------------------------------------------------------------------
> >>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> >>>> For additional commands, e-mail: dev-h...@commons.apache.org
> >>>>
> >>>>
> >>>
> >>>
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> >> For additional commands, e-mail: dev-h...@commons.apache.org
> >>
> >>
> >
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
>
--
http://people.apache.org/~britter/
http://www.systemoutprintln.de/
http://twitter.com/BenediktRitter
http://github.com/britter
