You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cxf.apache.org by ds...@apache.org on 2011/02/21 10:54:03 UTC
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
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/RMSoapInterceptor.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/cxf/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/cxf/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/cxf/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/RMSoapInterceptor.java
URL: http://svn.apache.org/viewvc/cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/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/RMSoapInterceptor.java (original)
+++ cxf/trunk/rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/soap/RMSoapInterceptor.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();
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();
>>
>
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 Daniel Kulp <dk...@apache.org>.
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