Hi Bill, Andreas, all: Just circling around to this. Thanks, Andreas, for your catch here. The default for AUTO_OPERATION_CLEANUP was definitely intended to be true, specifically to make avoiding the connection starvation problem as effortless (and backwards compatible) as possible. Please revert that part of the change if you would, Roy?
ServiceClients (and by association Stubs) are *not* meant to be thread-safe. There were a number of discussions about this in 2006, 2007, 2008... if the docs aren't crystal-clear on this then I definitely agree we should fix them. Re: the JavaDoc on cleanupTransport(), that's my bad - we do NOT build() the full Axiom tree by default with AUTO_OPERATION_CLEANUP, only when options.isCallTransportCleanup() is true. The build() actually happens in sendReceive() and the JavaDoc discussing it should be there, not on cleanupTransport(). I apologize for not neatening that up when I made the fix. I'd like to open a separate discussion as to how to make this whole cleanup issue work *right* for Axis2 1.6 (*), and deprecate the ways we've been doing it up until now, but for now we have three options - 1) use AUTO_OPERATION_CLEANUP by default which cleans up the *last* OperationContext each time you make a new one, 2) use options.setCallTransportCleanup(), which cleans up the *current* OperationContext at the end, accepting the build() limitations, and 3) shut off both of the above and clean up manually. Thanks, --Glen (*) IMHO there should be a way to have Axiom auto-trigger a cleanup if we reach the end of the inputStream, and if we don't ever read to the end for a given operation, we'll still need some automatic mechanism involving timeouts or heuristics.... Bill Nagy wrote: > 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(); >>> } >>> >>> >>> >
