关于同步访问，双重检查问题

Question

使用场景大致如下，有一些命名的资源，共多个线程使用，而这些资源在第一次使用时被初始化，并放入map中，这样其他线程就不用再初始化而可以从map中直接检索此资源使用。
实现如下：Java代码  
public class Resources {  
    private static Map resourceMap = Collections.synchronizedMap(new HashMap());  
    public static resource getResource(String name) {  
        Resource resource = (Resource) resourceMap.get(name);  
        if (resource == null) {  
            synchronized (resourceMap) {  
                resource = (Resource) resourceMap.get(name);  
                if (resource == null) {  
                    resource = createResource(name); //耗时的操作  
                    resourceMap.put(name, resource);  
                }  
            }  
        }  
    }  
}  
 但是有达人看了后说这有问题，类似于单态的双重检查问题，会有缺陷。我也查了一下，好似网上是说这么回事。在这里的朋友们，你们认为上面代码有问题吗？如果有问题有该如何改进了？

Answer 1

没看出问题，这个同单例不一样单例的问题在于给resourceMap赋值

Answer 2

to ticmy:
   这种写法没有问题吗？和单态有什么区别呀？to raistlic：
   直接加锁不是有性能低呀

Answer 3

public class Resources {  private static final Map<String, Resource> resourceMap =
          new HashMap<String, Resource>();  public static Resource getResource(String name) {    synchronized(resourceMap) {      Resource resource = resourceMap.get(name);
      
      if( resource == null ) {
        
        resource = createResource(name); //耗时的操作  
        resourceMap.put(name, resource);
      }
      return resource;
    }
  }
}

Answer 4

to raistlic：
  为啥呀？

Answer 5

我不知道 Collections.synchronizedMap() 返回的 map 是怎么实现线程安全的，但很可能是跟加锁差不多的成本，就是说，你第一句 Resource resource = (Resource) resourceMap.get(name);  很可能就已经跟直接加锁差不多了。我没有考证，说错勿怪。

Answer 6

下面这个用了双重检查，只对写操作加锁，并且 copy on write，利用 volatile reference 的特性(java 1.5+) 实现读操作的安全和 up to date，理论上应该是安全的，但是天知道是不是比直接加锁更贵。
import java.util.HashMap;
import java.util.Map;public class Resources {  private static volatile Map<String, Resource> resourceMap =
          new HashMap<String, Resource>();
  
  private static final Object writeLock = new Object();  public static Resource getResource(String name) {    Resource resource = resourceMap.get(name);
    
    if( resource == null ) {
      
      // sync & copy on write
      synchronized(writeLock) {
        
        resource = resourceMap.get(name);
        if( resource == null ) {          resource = createResource(name); //耗时的操作  
          Map<String, Resource> newMap = new HashMap<String, Resource>(resourceMap);
          newMap.put(name, resource);
          resourceMap = newMap;
        }
      }
    }
    return resource;
  }
}

Answer 7

单例的问题：
非volatile的可见性与指令重排序问题
jdk1.5之前volatile的语义不够强，指令重排序问题依然存在而你的resourceMap在类Resources初始化时已经做掉了，jvm保证初始化期间的线程安全。

Answer 8

8楼的代码只对写操作加锁重复的 String key 的读操作没有同步成本，所以如果读操作频繁的话，可能，——可能是比4楼那个直接加锁更有效率的

Answer 9

最前面这个if (resource == null)判断会不会出现线程问题？这样一个场景当线程1去map中找资源时发现他还没有初始化所以要去初始化，正在这时（线程1还没去实列化）线程2也来了然后也去访问这个资源发现他没有呗初始化，那他也要去初始化他，这样会不会导致一个资源被初始化两次？

Answer 10

楼主的代码没有问题，所谓的双重锁定问题，是因为JAVA内存模型的无序写入缺陷导致的，在一部分JVM上存在这样的情况： 
 
  public static Singleton getInstance()  
  {  
    if (instance == null)  
    {  
      synchronized(Singleton.class) {  //1  
        if (instance == null)          //2  
          instance = new Singleton();  //3  
      }  
    }  
    return instance;  
  }  
    
 
 
 上述代码可能会导致双重锁定失败。这是因为在//3处，在部分JVM中，instance会在new Singleton()执行之前就被置为非null了，从而导致双重锁定失败。具体请参见 
 http://www.ibm.com/developerworks/cn/java/j-dcl.html 
 
 但在你的例子中，没有类似静态变量instance的东西，resource是从map中取出来的，不存在无序写入的问题，不可能会锁定失败。 
 
 至于性能，因为大部分调用不会进入第一个 if (resource == null) ，所以问题不大。
 

Answer 11

查看了 Collections.synchronizedMap() 的源代码：public class Collections {  // ...
  public static <K,V> Map<K,V> synchronizedMap(Map<K,V> m) {
    return new SynchronizedMap<K,V>(m);
  }
  
  // ...
  private static class SynchronizedMap<K,V> implements Map<K,V>, Serializable {
    
    // use serialVersionUID from JDK 1.2.2 for interoperability
    private static final long serialVersionUID = 1978198479659022715L;    private final Map<K,V> m;     // Backing Map
    final Object mutex;          // Object on which to synchronize    SynchronizedMap(Map<K,V> m) {
      if (m==null)
        throw new NullPointerException();
      this.m = m;
      mutex = this;
    }
    
    // ...
    public V get(Object key) {
      synchronized(mutex) {return m.get(key);}
    }
    
    // ...
  }
}
也就是说，因为你用了 Collections.synchronizedMap(new HashMap()) 
你的每一次读操作的第一句：Resource resource = (Resource) resourceMap.get(name);都已经附带了加锁的成本，双重检查就变得没有意义。双重检查本来的目的就是“去除重复的读操作的同步成本”，
潜台词是“重复的读操作相对比较多，如果其中的同步成本能够去除，有利于提高效率”，————所以，可以参考8楼的实现。

Answer 12

一点拙见：
@see 《研磨设计模式》public class Resources {
private static String name;

public Resources(String name){
this.name = name;
}
private static class ResourcesHolder{
private static Map resourceMap = Collections.synchronizedMap(new HashMap());
private static Resources resources = createResources(name);
}

public static Resources getResources(String name){
return ResourcesHolder.resources;
}
}

Answer 13

你可以考虑resource的初始化在静态内部类之中，这样可以确保初始化一次，线程安全也得到确保（static成员变量只初始化一次的特性）

Answer 14

 private static Map resourceMap = Collections.synchronizedMap(new HashMap());
innerclass中这句为多余，删除后测试。