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(); > > } > > > > > > >