Ignoring the performance degradation for a moment, the
AUTO_OPERATION_CLEANUP feature had removed thread-safety from the
ServiceClient (i.e. if two threads invoke
serviceclient.createClient(...), there is the distinct possibility that,
before Roy's modification, the code would have cleaned up the transport
before the first invoking thread had finished.)

If the ServiceClient is not meant to be thread-safe, then this needs to
be explicitly stated in the JavaDoc and the JAX-WS code will need to be
reworked to take this into account.

If the ServiceClient is supposed to be thread-safe, then either the
default should remain as Roy has set it (i.e. to be 'false') or the
cleanup code needs to be rewritten so as not to cause threading issues.

-Bill


On Thu, 2009-11-12 at 17:44 -0600, Roy Wood wrote:
> 
> Andreas, 
> Yes, I agree...thanks for the correction. I went back and did a quick
> search on AUTO_OPERATION_CLEANUP and didn't find any intent on what
> the 
> actual default should be, other than the original code using a default
> of 'true'. 
> 
> This property came to our attention when we ran into a threading
> problem in one of our test cases. By setting the default to false,
> thus disabling the call to 
> 'cleanupTransport' in createClient, the threading problem disappeared.
> I'm also a bit concerned about the performance warning in
> cleanupTransport 
> javadocs. For that reason, as well as, providing a degree backwards
> compatability,  I would like to propose that the default for this
> property be 'false'. 
> 
> Roy A. Wood, Jr.
> WebSphere Development - Web Services
> wood...@us.ibm.com
> 512-286-9307  T/L:363-9307
> 11501 Burnet Road,  Austin TX   78758 (Internal ZIP: 9372) 
> 
> 
> 
> Re: svn commit: r835113
> - 
> /webservices/axis2/trunk/java/modules/kernel/src/org/apache/axis2/client/ServiceClient.java
> 
> Andreas
> Veithen 
> to: 
> axis-dev 
> 11/12/2009 03:30 PM
> 
> Cc: 
> Glen
> Daniels
> 
> Please respond to
> axis-dev 
> 
> 
> 
> 
> 
> 
> ______________________________________________________________________
> 
> 
> 
> That is not correct. The entire Javadoc of the cleanupTransport method
> was written before the introduction of the AUTO_OPERATION_CLEANUP
> property. It only refers to "callTransportCleanup", which is a
> different property. Since the AUTO_OPERATION_CLEANUP feature is
> something that has been recently introduced by Glen for the 1.5.1
> release, it would be good to start a discussion to get his feedback if
> you think that the default value should be different for the next
> release.
> 
> Andreas
> 
> On Wed, Nov 11, 2009 at 23:54,  <wood...@apache.org> wrote:
> > Author: woodroy
> > Date: Wed Nov 11 22:54:35 2009
> > New Revision: 835113
> >
> > URL: http://svn.apache.org/viewvc?rev=835113&view=rev
> > Log:
> > Use proper default value of AUTO_OPERATION_CLEANUP property
> >
> > Javadoc for cleanupTransport states the default value is 'false'
> >
> > Modified:
> >
>  
> webservices/axis2/trunk/java/modules/kernel/src/org/apache/axis2/client/ServiceClient.java
> >
> > Modified:
> webservices/axis2/trunk/java/modules/kernel/src/org/apache/axis2/client/ServiceClient.java
> > URL:
> http://svn.apache.org/viewvc/webservices/axis2/trunk/java/modules/kernel/src/org/apache/axis2/client/ServiceClient.java?rev=835113&r1=835112&r2=835113&view=diff
> >
> ==============================================================================
> > ---
> webservices/axis2/trunk/java/modules/kernel/src/org/apache/axis2/client/ServiceClient.java
>  (original)
> > +++
> webservices/axis2/trunk/java/modules/kernel/src/org/apache/axis2/client/ServiceClient.java
>  Wed Nov 11 22:54:35 2009
> > @@ -660,7 +660,7 @@
> >     public OperationClient createClient(QName operationQName) throws
> AxisFault {
> >         // If we're configured to do so, clean up the last
> OperationContext (thus
> >         // releasing its resources) each time we create a new one.
> > -        if
> (JavaUtils.isTrue(getOptions().getProperty(AUTO_OPERATION_CLEANUP),
> true) &&
> > +        if
> (JavaUtils.isTrue(getOptions().getProperty(AUTO_OPERATION_CLEANUP),
> false) &&
> >                 !getOptions().isUseSeparateListener()) {
> >             cleanupTransport();
> >         }
> >
> >
> >
> 

Reply via email to