synapse-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andreas Veithen <andreas.veit...@gmail.com>
Subject Re: svn commit: r1610262 - /synapse/trunk/java/modules/transports/core/nhttp/src/main/java/org/apache/synapse/transport/passthru/connections/TargetConnections.java
Date Sun, 13 Jul 2014 18:48:02 GMT
That doesn't look thread safe:
* Calling containsKey is not the same as a null check.
* "double checked locking" is an anti-pattern because it is unsafe.
* In addition to that, you added an unsynchronized call to the get method.

Andreas

On Sun, Jul 13, 2014 at 7:15 PM,  <uswick@apache.org> wrote:
> Author: uswick
> Date: Sun Jul 13 18:14:59 2014
> New Revision: 1610262
>
> URL: http://svn.apache.org/r1610262
> Log:
> avoid sync overhead each time null is checked - use double checked locking
>
> Modified:
>     synapse/trunk/java/modules/transports/core/nhttp/src/main/java/org/apache/synapse/transport/passthru/connections/TargetConnections.java
>
> Modified: synapse/trunk/java/modules/transports/core/nhttp/src/main/java/org/apache/synapse/transport/passthru/connections/TargetConnections.java
> URL: http://svn.apache.org/viewvc/synapse/trunk/java/modules/transports/core/nhttp/src/main/java/org/apache/synapse/transport/passthru/connections/TargetConnections.java?rev=1610262&r1=1610261&r2=1610262&view=diff
> ==============================================================================
> --- synapse/trunk/java/modules/transports/core/nhttp/src/main/java/org/apache/synapse/transport/passthru/connections/TargetConnections.java
(original)
> +++ synapse/trunk/java/modules/transports/core/nhttp/src/main/java/org/apache/synapse/transport/passthru/connections/TargetConnections.java
Sun Jul 13 18:14:59 2014
> @@ -205,16 +205,16 @@ public class TargetConnections {
>
>      private HostConnections getConnectionPool(String host, int port) {
>          String key = host + ":" + port;
> -        HostConnections pool;
> -        synchronized (poolMap) {
> -            // see weather a pool already exists for this host:port
> -            pool = poolMap.get(key);
> -            if (pool == null) {
> -                pool = new HostConnections(host, port, maxConnections);
> -                poolMap.put(key, pool);
> +        if (!poolMap.containsKey(key)) {
> +            synchronized (poolMap) {
> +                // see weather a pool already exists for this host:port
> +                if (!poolMap.containsKey(key)) {
> +                    HostConnections pool = new HostConnections(host, port, maxConnections);
> +                    poolMap.put(key, pool);
> +                }
>              }
>          }
> -        return pool;
> +        return poolMap.get(key);
>      }
>
>  }
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@synapse.apache.org
For additional commands, e-mail: dev-help@synapse.apache.org


Mime
View raw message