You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cxf.apache.org by Daniel Kulp <dk...@apache.org> on 2011/02/21 19:15:31 UTC
Re: svn commit: r1072914 - in /cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm: Destination.java RMInInterceptor.java RMProperties.java soap/RMSoapInterceptor.java
Dennis,
Curiosity:
Would just switching from ArrayList and HashMap to CopyOnWriteArrayList and
ConcurrentHashMap be a better solution for this? In general, I'm not a fan
of sticking synchronized blocks all over the place if we don't need to.
Dan
On Monday 21 February 2011 4:54:03 AM dsosnoski@apache.org wrote:
> Author: dsosnoski
> Date: Mon Feb 21 09:54:02 2011
> New Revision: 1072914
>
> URL: http://svn.apache.org/viewvc?rev=1072914&view=rev
> Log:
> Sychronize use of collections in WS-RM acknowledgement processing
> (CXF-3273)
>
> Modified:
> cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/Destination.java
>
> cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/RMInInterceptor.java
> cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/RMProperties.java
> cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/soap/RMSoapIntercept
> or.java
>
> Modified:
> cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/Destination.java
> URL:
> http://svn.apache.org/viewvc/cxf/trunk/rt/ws/rm/src/main/java/org/apache/c
> xf/ws/rm/Destination.java?rev=1072914&r1=1072913&r2=1072914&view=diff
> ==========================================================================
> ==== ---
> cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/Destination.java
> (original) +++
> cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/Destination.java Mon
> Feb 21 09:54:02 2011 @@ -140,13 +140,14 @@ public class Destination
> extends Abstrac
> if (null == ars) {
> return;
> }
> - for (AckRequestedType ar : ars) {
> - Identifier id = ar.getIdentifier();
> - DestinationSequence seq = getSequence(id);
> - if (null == seq) {
> - continue;
> + synchronized (ars) {
> + for (AckRequestedType ar : ars) {
> + Identifier id = ar.getIdentifier();
> + DestinationSequence seq = getSequence(id);
> + if (null != seq) {
> + ackImmediately(seq, message);
> + }
> }
> - ackImmediately(seq, message);
> }
> }
>
>
> Modified:
> cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/RMInInterceptor.java
> URL:
> http://svn.apache.org/viewvc/cxf/trunk/rt/ws/rm/src/main/java/org/apache/c
> xf/ws/rm/RMInInterceptor.java?rev=1072914&r1=1072913&r2=1072914&view=diff
> ==========================================================================
> ==== ---
> cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/RMInInterceptor.java
> (original) +++
> cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/RMInInterceptor.java
> Mon Feb 21 09:54:02 2011 @@ -123,13 +123,15 @@ public class
> RMInInterceptor extends Abs
>
> Collection<SequenceAcknowledgement> acks = rmps.getAcks();
> if (null != acks) {
> - for (SequenceAcknowledgement ack : acks) {
> - Identifier id = ack.getIdentifier();
> - SourceSequence ss = source.getSequence(id);
> - if (null != ss) {
> - ss.setAcknowledged(ack);
> - } else {
> - throw (new
> SequenceFaultFactory()).createUnknownSequenceFault(id); +
> synchronized (acks) {
> + for (SequenceAcknowledgement ack : acks) {
> + Identifier id = ack.getIdentifier();
> + SourceSequence ss = source.getSequence(id);
> + if (null != ss) {
> + ss.setAcknowledged(ack);
> + } else {
> + throw (new
> SequenceFaultFactory()).createUnknownSequenceFault(id); +
> }
> }
> }
> }
>
> Modified:
> cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/RMProperties.java
> URL:
> http://svn.apache.org/viewvc/cxf/trunk/rt/ws/rm/src/main/java/org/apache/c
> xf/ws/rm/RMProperties.java?rev=1072914&r1=1072913&r2=1072914&view=diff
> ==========================================================================
> ==== ---
> cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/RMProperties.java
> (original) +++
> cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/RMProperties.java
> Mon Feb 21 09:54:02 2011 @@ -40,11 +40,15 @@ public class RMProperties {
> }
>
> public void setAcks(Collection<SequenceAcknowledgement> a) {
> - acks = a;
> + synchronized (a) {
> + acks = a;
> + }
> }
>
> public void setAcksRequested(Collection<AckRequestedType> ar) {
> - acksRequested = ar;
> + synchronized (ar) {
> + acksRequested = ar;
> + }
> }
>
> public void setSequence(SequenceType s) {
> @@ -65,9 +69,11 @@ public class RMProperties {
> if (null == acks) {
> acks = new ArrayList<SequenceAcknowledgement>();
> }
> - SequenceAcknowledgement ack = seq.getAcknowledgment();
> - acks.add(ack);
> - seq.acknowledgmentSent();
> + synchronized (acks) {
> + SequenceAcknowledgement ack = seq.getAcknowledgment();
> + acks.add(ack);
> + seq.acknowledgmentSent();
> + }
> }
>
> }
>
> Modified:
> cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/soap/RMSoapIntercept
> or.java URL:
> http://svn.apache.org/viewvc/cxf/trunk/rt/ws/rm/src/main/java/org/apache/c
> xf/ws/rm/soap/RMSoapInterceptor.java?rev=1072914&r1=1072913&r2=1072914&view
> =diff
> ==========================================================================
> ==== ---
> cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/soap/RMSoapIntercept
> or.java (original) +++
> cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/soap/RMSoapIntercept
> or.java Mon Feb 21 09:54:02 2011 @@ -202,22 +202,26 @@ public class
> RMSoapInterceptor extends A
> }
> Collection<SequenceAcknowledgement> acks = rmps.getAcks();
> if (null != acks) {
> - for (SequenceAcknowledgement ack : acks) {
> - encodeProperty(ack,
> - RMConstants.getSequenceAckQName(),
> - SequenceAcknowledgement.class,
> - hdr,
> - marshaller);
> + synchronized (acks) {
> + for (SequenceAcknowledgement ack : acks) {
> + encodeProperty(ack,
> + RMConstants.getSequenceAckQName(),
> + SequenceAcknowledgement.class,
> + hdr,
> + marshaller);
> + }
> }
> }
> Collection<AckRequestedType> requested =
> rmps.getAcksRequested(); if (null != requested) {
> - for (AckRequestedType ar : requested) {
> - encodeProperty(ar,
> - RMConstants.getAckRequestedQName(),
> - AckRequestedType.class,
> - hdr,
> - marshaller);
> + synchronized (requested) {
> + for (AckRequestedType ar : requested) {
> + encodeProperty(ar,
> + RMConstants.getAckRequestedQName(),
> + AckRequestedType.class,
> + hdr,
> + marshaller);
> + }
> }
> }
> Node node = hdr.getFirstChild();
--
Daniel Kulp
dkulp@apache.org
http://dankulp.com/blog
Talend - http://www.talend.com
Re: svn commit: r1072914 - in /cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm:
Destination.java RMInInterceptor.java RMProperties.java soap/RMSoapInterceptor.java
Posted by Dennis Sosnoski <dm...@sosnoski.com>.
Good catch - yes, I think this is a better solution. We're actually only
using lists, so changing to CopyOnWriteArrayList should work correctly.
I'll just copy the Collection passed in on the set methods to a new
CopyOnWriteArrayList and save that, so that we can be sure the correct
type of Collection is being used without any impact to code outside
RMProperties. As it is now, only the Acks collection is actually being
modified after creation anyway.
- Dennis
On 02/22/2011 07:15 AM, Daniel Kulp wrote:
> Dennis,
>
> Curiosity:
> Would just switching from ArrayList and HashMap to CopyOnWriteArrayList and
> ConcurrentHashMap be a better solution for this? In general, I'm not a fan
> of sticking synchronized blocks all over the place if we don't need to.
>
> Dan
>
>
>
>
> On Monday 21 February 2011 4:54:03 AM dsosnoski@apache.org wrote:
>
>> Author: dsosnoski
>> Date: Mon Feb 21 09:54:02 2011
>> New Revision: 1072914
>>
>> URL: http://svn.apache.org/viewvc?rev=1072914&view=rev
>> Log:
>> Sychronize use of collections in WS-RM acknowledgement processing
>> (CXF-3273)
>>
>> Modified:
>> cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/Destination.java
>>
>> cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/RMInInterceptor.java
>> cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/RMProperties.java
>> cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/soap/RMSoapIntercept
>> or.java
>>
>> Modified:
>> cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/Destination.java
>> URL:
>> http://svn.apache.org/viewvc/cxf/trunk/rt/ws/rm/src/main/java/org/apache/c
>> xf/ws/rm/Destination.java?rev=1072914&r1=1072913&r2=1072914&view=diff
>> ==========================================================================
>> ==== ---
>> cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/Destination.java
>> (original) +++
>> cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/Destination.java Mon
>> Feb 21 09:54:02 2011 @@ -140,13 +140,14 @@ public class Destination
>> extends Abstrac
>> if (null == ars) {
>> return;
>> }
>> - for (AckRequestedType ar : ars) {
>> - Identifier id = ar.getIdentifier();
>> - DestinationSequence seq = getSequence(id);
>> - if (null == seq) {
>> - continue;
>> + synchronized (ars) {
>> + for (AckRequestedType ar : ars) {
>> + Identifier id = ar.getIdentifier();
>> + DestinationSequence seq = getSequence(id);
>> + if (null != seq) {
>> + ackImmediately(seq, message);
>> + }
>> }
>> - ackImmediately(seq, message);
>> }
>> }
>>
>>
>> Modified:
>> cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/RMInInterceptor.java
>> URL:
>> http://svn.apache.org/viewvc/cxf/trunk/rt/ws/rm/src/main/java/org/apache/c
>> xf/ws/rm/RMInInterceptor.java?rev=1072914&r1=1072913&r2=1072914&view=diff
>> ==========================================================================
>> ==== ---
>> cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/RMInInterceptor.java
>> (original) +++
>> cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/RMInInterceptor.java
>> Mon Feb 21 09:54:02 2011 @@ -123,13 +123,15 @@ public class
>> RMInInterceptor extends Abs
>>
>> Collection<SequenceAcknowledgement> acks = rmps.getAcks();
>> if (null != acks) {
>> - for (SequenceAcknowledgement ack : acks) {
>> - Identifier id = ack.getIdentifier();
>> - SourceSequence ss = source.getSequence(id);
>> - if (null != ss) {
>> - ss.setAcknowledged(ack);
>> - } else {
>> - throw (new
>> SequenceFaultFactory()).createUnknownSequenceFault(id); +
>> synchronized (acks) {
>> + for (SequenceAcknowledgement ack : acks) {
>> + Identifier id = ack.getIdentifier();
>> + SourceSequence ss = source.getSequence(id);
>> + if (null != ss) {
>> + ss.setAcknowledged(ack);
>> + } else {
>> + throw (new
>> SequenceFaultFactory()).createUnknownSequenceFault(id); +
>> }
>> }
>> }
>> }
>>
>> Modified:
>> cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/RMProperties.java
>> URL:
>> http://svn.apache.org/viewvc/cxf/trunk/rt/ws/rm/src/main/java/org/apache/c
>> xf/ws/rm/RMProperties.java?rev=1072914&r1=1072913&r2=1072914&view=diff
>> ==========================================================================
>> ==== ---
>> cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/RMProperties.java
>> (original) +++
>> cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/RMProperties.java
>> Mon Feb 21 09:54:02 2011 @@ -40,11 +40,15 @@ public class RMProperties {
>> }
>>
>> public void setAcks(Collection<SequenceAcknowledgement> a) {
>> - acks = a;
>> + synchronized (a) {
>> + acks = a;
>> + }
>> }
>>
>> public void setAcksRequested(Collection<AckRequestedType> ar) {
>> - acksRequested = ar;
>> + synchronized (ar) {
>> + acksRequested = ar;
>> + }
>> }
>>
>> public void setSequence(SequenceType s) {
>> @@ -65,9 +69,11 @@ public class RMProperties {
>> if (null == acks) {
>> acks = new ArrayList<SequenceAcknowledgement>();
>> }
>> - SequenceAcknowledgement ack = seq.getAcknowledgment();
>> - acks.add(ack);
>> - seq.acknowledgmentSent();
>> + synchronized (acks) {
>> + SequenceAcknowledgement ack = seq.getAcknowledgment();
>> + acks.add(ack);
>> + seq.acknowledgmentSent();
>> + }
>> }
>>
>> }
>>
>> Modified:
>> cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/soap/RMSoapIntercept
>> or.java URL:
>> http://svn.apache.org/viewvc/cxf/trunk/rt/ws/rm/src/main/java/org/apache/c
>> xf/ws/rm/soap/RMSoapInterceptor.java?rev=1072914&r1=1072913&r2=1072914&view
>> =diff
>> ==========================================================================
>> ==== ---
>> cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/soap/RMSoapIntercept
>> or.java (original) +++
>> cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/soap/RMSoapIntercept
>> or.java Mon Feb 21 09:54:02 2011 @@ -202,22 +202,26 @@ public class
>> RMSoapInterceptor extends A
>> }
>> Collection<SequenceAcknowledgement> acks = rmps.getAcks();
>> if (null != acks) {
>> - for (SequenceAcknowledgement ack : acks) {
>> - encodeProperty(ack,
>> - RMConstants.getSequenceAckQName(),
>> - SequenceAcknowledgement.class,
>> - hdr,
>> - marshaller);
>> + synchronized (acks) {
>> + for (SequenceAcknowledgement ack : acks) {
>> + encodeProperty(ack,
>> + RMConstants.getSequenceAckQName(),
>> + SequenceAcknowledgement.class,
>> + hdr,
>> + marshaller);
>> + }
>> }
>> }
>> Collection<AckRequestedType> requested =
>> rmps.getAcksRequested(); if (null != requested) {
>> - for (AckRequestedType ar : requested) {
>> - encodeProperty(ar,
>> - RMConstants.getAckRequestedQName(),
>> - AckRequestedType.class,
>> - hdr,
>> - marshaller);
>> + synchronized (requested) {
>> + for (AckRequestedType ar : requested) {
>> + encodeProperty(ar,
>> + RMConstants.getAckRequestedQName(),
>> + AckRequestedType.class,
>> + hdr,
>> + marshaller);
>> + }
>> }
>> }
>> Node node = hdr.getFirstChild();
>>
>