You are viewing a plain text version of this content. The canonical link for it is here.
Posted to java-dev@axis.apache.org by Andreas Veithen <an...@gmail.com> on 2010/12/21 22:14:41 UTC
Re: svn commit: r1051511 - /axis/axis2/java/core/trunk/modules/kernel/src/org/apache/axis2/description/ClientUtils.java
I'm not sure that this is a valid change:
1. The fact that a method only reads information doesn't automatically
imply that it doesn't need synchronization. See the Java Memory Model
for an explanation why this is an incorrect assumption.
2. The synchronization was added in r387691 to fix a "synchronization
problem in client side" [1]. I don't know what the problem was and if
that was the right fix, but reverting it definitely requires some more
thinking.
3. At least the inferInTransport may modify the state of the system,
namely add a transport listener. Therefore it is not true to say that
these methods only read information.
Andreas
[1] http://svn.apache.org/viewvc?view=revision&revision=387691
On Tue, Dec 21, 2010 at 15:44, <su...@apache.org> wrote:
> Author: supun
> Date: Tue Dec 21 14:44:27 2010
> New Revision: 1051511
>
> URL: http://svn.apache.org/viewvc?rev=1051511&view=rev
> Log:
> removing un-necessary synchronizations as these method only reads information
>
> Modified:
> axis/axis2/java/core/trunk/modules/kernel/src/org/apache/axis2/description/ClientUtils.java
>
> Modified: axis/axis2/java/core/trunk/modules/kernel/src/org/apache/axis2/description/ClientUtils.java
> URL: http://svn.apache.org/viewvc/axis/axis2/java/core/trunk/modules/kernel/src/org/apache/axis2/description/ClientUtils.java?rev=1051511&r1=1051510&r2=1051511&view=diff
> ==============================================================================
> --- axis/axis2/java/core/trunk/modules/kernel/src/org/apache/axis2/description/ClientUtils.java (original)
> +++ axis/axis2/java/core/trunk/modules/kernel/src/org/apache/axis2/description/ClientUtils.java Tue Dec 21 14:44:27 2010
> @@ -41,7 +41,7 @@ public class ClientUtils {
>
> private static final Log log = LogFactory.getLog(ClientUtils.class);
>
> - public static synchronized TransportOutDescription inferOutTransport(AxisConfiguration ac,
> + public static TransportOutDescription inferOutTransport(AxisConfiguration ac,
> EndpointReference epr,
> MessageContext msgctx)
> throws AxisFault {
> @@ -83,7 +83,7 @@ public class ClientUtils {
> }
> }
>
> - public static synchronized TransportInDescription inferInTransport(AxisConfiguration ac,
> + public static TransportInDescription inferInTransport(AxisConfiguration ac,
> Options options,
> MessageContext msgCtxt)
> throws AxisFault {
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@axis.apache.org
For additional commands, e-mail: java-dev-help@axis.apache.org
Re: svn commit: r1051511 - /axis/axis2/java/core/trunk/modules/kernel/src/org/apache/axis2/description/ClientUtils.java
Posted by Supun Kamburugamuva <su...@gmail.com>.
Hi Andreas,
I did revert the change for the function where it add the transport
listener. But this method seems to be wrong because it says it
inferTransport but actually it is doing write operations as well. I'll
try to re-factor this method properly if possible.
The other method, we are using in synapse and I did a load test
removing the synchronization and it works without issue.
Thanks,
Supun..
On Wed, Dec 22, 2010 at 2:44 AM, Andreas Veithen
<an...@gmail.com> wrote:
> I'm not sure that this is a valid change:
>
> 1. The fact that a method only reads information doesn't automatically
> imply that it doesn't need synchronization. See the Java Memory Model
> for an explanation why this is an incorrect assumption.
> 2. The synchronization was added in r387691 to fix a "synchronization
> problem in client side" [1]. I don't know what the problem was and if
> that was the right fix, but reverting it definitely requires some more
> thinking.
> 3. At least the inferInTransport may modify the state of the system,
> namely add a transport listener. Therefore it is not true to say that
> these methods only read information.
>
> Andreas
>
> [1] http://svn.apache.org/viewvc?view=revision&revision=387691
>
> On Tue, Dec 21, 2010 at 15:44, <su...@apache.org> wrote:
>> Author: supun
>> Date: Tue Dec 21 14:44:27 2010
>> New Revision: 1051511
>>
>> URL: http://svn.apache.org/viewvc?rev=1051511&view=rev
>> Log:
>> removing un-necessary synchronizations as these method only reads information
>>
>> Modified:
>> axis/axis2/java/core/trunk/modules/kernel/src/org/apache/axis2/description/ClientUtils.java
>>
>> Modified: axis/axis2/java/core/trunk/modules/kernel/src/org/apache/axis2/description/ClientUtils.java
>> URL: http://svn.apache.org/viewvc/axis/axis2/java/core/trunk/modules/kernel/src/org/apache/axis2/description/ClientUtils.java?rev=1051511&r1=1051510&r2=1051511&view=diff
>> ==============================================================================
>> --- axis/axis2/java/core/trunk/modules/kernel/src/org/apache/axis2/description/ClientUtils.java (original)
>> +++ axis/axis2/java/core/trunk/modules/kernel/src/org/apache/axis2/description/ClientUtils.java Tue Dec 21 14:44:27 2010
>> @@ -41,7 +41,7 @@ public class ClientUtils {
>>
>> private static final Log log = LogFactory.getLog(ClientUtils.class);
>>
>> - public static synchronized TransportOutDescription inferOutTransport(AxisConfiguration ac,
>> + public static TransportOutDescription inferOutTransport(AxisConfiguration ac,
>> EndpointReference epr,
>> MessageContext msgctx)
>> throws AxisFault {
>> @@ -83,7 +83,7 @@ public class ClientUtils {
>> }
>> }
>>
>> - public static synchronized TransportInDescription inferInTransport(AxisConfiguration ac,
>> + public static TransportInDescription inferInTransport(AxisConfiguration ac,
>> Options options,
>> MessageContext msgCtxt)
>> throws AxisFault {
>>
>>
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: java-dev-unsubscribe@axis.apache.org
> For additional commands, e-mail: java-dev-help@axis.apache.org
>
>
--
Technical Lead, WSO2 Inc
http://wso2.org
supunk.blogspot.com
---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@axis.apache.org
For additional commands, e-mail: java-dev-help@axis.apache.org