axis-java-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Roy Wood <wood...@us.ibm.com>
Subject Re: [Axis2] Re: svn commit: r835113 - /webservices/axis2/trunk/java/modules/kernel/src/org/apache/axis2/client/ServiceClient.java
Date Fri, 13 Nov 2009 20:00:58 GMT
All,

Thanks for the discussion and insight on this. I'll go ahead and revert 
this change and we'll look at doing a more suitable change in JAX-WS.

Roy A. Wood, Jr.
WebSphere Development - Web Services
woodroy@us.ibm.com
512-286-9307  T/L:363-9307
11501 Burnet Road,  Austin TX   78758 (Internal ZIP: 9372)



From:
Glen Daniels <glen@thoughtcraft.com>
To:
axis-dev@ws.apache.org
Date:
11/13/2009 12:47 PM
Subject:
Re: [Axis2] Re: svn commit: r835113     - 
/webservices/axis2/trunk/java/modules/kernel/src/org/apache/axis2/client/ServiceClient.java



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



Mime
View raw message