OK, then I will add one to my last change.
This shows again that exceptions are a kind of goto :/
Jacques
On Thursday, December 19, 2013 12:24 AM Adrian Crum
adrian.c...@sandglass-software.com
> In the past, some committers have complained that the source of the
> exception is obscured by calling code (which may quietly swallow it), so
> the preferred method is to log the exception before throwing it.
>
> Adrian Crum
> Sandglass Software
> www.sandglass-software.com
>
> On 12/18/2013 6:16 PM, Jacques Le Roux wrote:
>> BTW Adrian,
>>
>> Do we really need to have a pattern like
>> Debug.logError(e, "Exception thrown while creating
>> ConnectionFactoryInterface instance: ", module);
>> throw new IllegalStateException("Error loading
>> ConnectionFactoryInterface class: " + e);
>>
>> throw new IllegalStateException("Error loading
>> ConnectionFactoryInterface class: " + e);
>> would not be enough?
>>
>> Jacques
>>
>> On Wednesday, December 18, 2013 10:57 PM Jacques Le Roux
>> jacques.le.r...@les7arts.com
>>> Indeed, done at r1552120
>>>
>>> Jacques
>>>
>>> On Wednesday, December 18, 2013 9:33 PM adrian.c...@sandglass-software.com
>>> adrian.c...@sandglass-software.com
>>>> Jacques,
>>>>
>>>> Thank you for working on this.
>>>>
>>>> It might be best to have the AtomicReference update outside the
>>>> try-catch block. So...
>>>>
>>>> 1. Try to create a new sequencer.
>>>> 2. If successful, update the AtomicReference.
>>>>
>>>> The end result will be the same, but (from my perspective) it will be
>>>> easier to read and understand.
>>>>
>>>> -Adrian
>>>>
>>>>
>>>> Quoting jler...@apache.org:
>>>>
>>>>> Author: jleroux
>>>>> Date: Wed Dec 18 19:46:50 2013
>>>>> New Revision: 1552073
>>>>>
>>>>> URL: http://svn.apache.org/r1552073
>>>>> Log:
>>>>> Fixes <<FIXME: Replace DCL code with AtomicReference>>
>>>>>
>>>>> Modified:
>>>>>
>>>>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java
>>>>>
>>>>> Modified:
>>>>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java
>>>>> URL:
>>>>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java?rev=1552073&r1=1552072&r2=1552073&view=diff
>>>>> ==============================================================================
>>>>> ---
>>>>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java
>>>>> (original)
>>>>> +++
>>>>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java
>>>>> Wed
>>>>> Dec 18 19:46:50 2013
>>>>> @@ -32,6 +32,7 @@ import java.util.Set;
>>>>> import java.util.concurrent.Callable;
>>>>> import java.util.concurrent.Future;
>>>>> import java.util.concurrent.LinkedBlockingDeque;
>>>>> +import java.util.concurrent.atomic.AtomicReference;
>>>>> import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
>>>>>
>>>>> import javax.xml.parsers.ParserConfigurationException;
>>>>> @@ -105,7 +106,7 @@ public class GenericDelegator implements
>>>>> protected DistributedCacheClear distributedCacheClear = null;
>>>>> protected boolean warnNoEcaHandler = false;
>>>>> protected EntityEcaHandler<?> entityEcaHandler = null;
>>>>> - protected SequenceUtil sequencer = null;
>>>>> + protected final AtomicReference<SequenceUtil>
>>>>> AtomicRefSequencer = new AtomicReference<SequenceUtil>(null);
>>>>> protected EntityCrypto crypto = null;
>>>>>
>>>>> /** A ThreadLocal variable to allow other methods to specify a
>>>>> user identifier (usually the userLoginId, though technically the
>>>>> Entity Engine doesn't know anything about the UserLogin entity) */
>>>>> @@ -791,7 +792,7 @@ public class GenericDelegator implements
>>>>> }
>>>>>
>>>>> // found an existing value... was probably a
>>>>> duplicate key, so clean things up and try again
>>>>> -
>>>>> this.sequencer.forceBankRefresh(value.getEntityName(), 1);
>>>>> +
>>>>> this.AtomicRefSequencer.get().forceBankRefresh(value.getEntityName(),
>>>>> 1);
>>>>>
>>>>> value.setNextSeqId();
>>>>> value = helper.create(value);
>>>>> @@ -2389,13 +2390,16 @@ public class GenericDelegator implements
>>>>> beganTransaction = TransactionUtil.begin();
>>>>> }
>>>>>
>>>>> - // FIXME: Replace DCL code with AtomicReference
>>>>> + SequenceUtil sequencer = this.AtomicRefSequencer.get();
>>>>> if (sequencer == null) {
>>>>> - synchronized (this) {
>>>>> - if (sequencer == null) {
>>>>> - ModelEntity seqEntity =
>>>>> this.getModelEntity("SequenceValueItem");
>>>>> - sequencer = new SequenceUtil(this,
>>>>> this.getEntityHelperInfo("SequenceValueItem"), seqEntity, "seqName",
>>>>> "seqId");
>>>>> + try {
>>>>> + ModelEntity seqEntity =
>>>>> this.getModelEntity("SequenceValueItem");
>>>>> + sequencer = new SequenceUtil(this,
>>>>> this.getEntityHelperInfo("SequenceValueItem"), seqEntity, "seqName",
>>>>> "seqId");
>>>>> + if (!AtomicRefSequencer.compareAndSet(null,
>>>>> sequencer)) {
>>>>> + sequencer = this.AtomicRefSequencer.get();
>>>>> }
>>>>> + } catch (Exception e) {
>>>>> + throw new IllegalStateException("Exception
>>>>> thrown while creating AtomicReference<SequenceUtil>: " + e);
>>>>> }
>>>>> }
>>>>>
>>>>> @@ -2421,14 +2425,14 @@ public class GenericDelegator implements
>>>>> * @see
>>>>> org.ofbiz.entity.Delegator#setSequencer(org.ofbiz.entity.util.SequenceUtil)
>>>>> */
>>>>> public void setSequencer(SequenceUtil sequencer) {
>>>>> - this.sequencer = sequencer;
>>>>> + this.AtomicRefSequencer.set(sequencer);
>>>>> }
>>>>>
>>>>> /* (non-Javadoc)
>>>>> * @see org.ofbiz.entity.Delegator#refreshSequencer()
>>>>> */
>>>>> public void refreshSequencer() {
>>>>> - this.sequencer = null;
>>>>> + this.AtomicRefSequencer.set(null);
>>>>> }
