cxf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dennis Sosnoski <...@sosnoski.com>
Subject 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
Date Mon, 21 Feb 2011 21:18:01 GMT
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();
>>     
>   

Mime
View raw message