You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by jl...@apache.org on 2013/12/18 20:46:50 UTC
svn commit: r1552073 -
/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java
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);
}
Re: svn commit: r1552073 -
/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java
Posted by Jacques Le Roux <ja...@les7arts.com>.
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.crum@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.roux@les7arts.com
>>> Indeed, done at r1552120
>>>
>>> Jacques
>>>
>>> On Wednesday, December 18, 2013 9:33 PM adrian.crum@sandglass-software.com adrian.crum@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 jleroux@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);
>>>>> }
Re: svn commit: r1552073 - /ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java
Posted by Adrian Crum <ad...@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.roux@les7arts.com
>> Indeed, done at r1552120
>>
>> Jacques
>>
>> On Wednesday, December 18, 2013 9:33 PM adrian.crum@sandglass-software.com adrian.crum@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 jleroux@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);
>>>> }
Re: svn commit: r1552073 -
/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java
Posted by Jacques Le Roux <ja...@les7arts.com>.
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.roux@les7arts.com
> Indeed, done at r1552120
>
> Jacques
>
> On Wednesday, December 18, 2013 9:33 PM adrian.crum@sandglass-software.com adrian.crum@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 jleroux@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);
>>> }
Re: svn commit: r1552073 -
/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java
Posted by Jacques Le Roux <ja...@les7arts.com>.
Indeed, done at r1552120
Jacques
On Wednesday, December 18, 2013 9:33 PM adrian.crum@sandglass-software.com adrian.crum@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 jleroux@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);
>> }
Re: svn commit: r1552073 -
/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java
Posted by ad...@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 jleroux@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);
> }
>
>
>
>
>