synapse-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Udayanga Wickramasinghe <mastershield2...@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 Mon, 14 Jul 2014 06:19:17 GMT
On Sun, Jul 13, 2014 at 2:48 PM, Andreas Veithen <andreas.veithen@gmail.com>
wrote:

> That doesn't look thread safe:
> * Calling containsKey is not the same as a null check.
>
true, but no one modifies the key/value pair once it is written so checking
key is essentially same as checking value.


> * "double checked locking" is an anti-pattern because it is unsafe.
> * In addition to that, you added an unsynchronized call to the get method.
>

I am aware of the java memory model and partial initialization problem with
DCL. However I do not necessarily believe it is an issue for this specific
scenario. First we check for the key in map#containsKey() which returns an
boolean which is a 32 bit primitive. 32 bit primitives are atomic and do
not cause partial writes within JVM. Secondly concurrent hash map and
related methods are synchronized which guarantees a read/write memory
barrier within the call block. Therefore I think it should be the case that
the map has been atomically synchronized (or atomically not) for all its
state between memory/local cache once the check is made (even if we assume
that this is not the case final map#get() should execute the respective
memory barriers. This will synchronize members between threads so that
existence of any partial data of the map is eliminated ).


> 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
>
>


-- 
http://www.udayangawiki.blogspot.com

Mime
View raw message