From 5159aab259b67108f9adcdffd863b5c7d58af261 Mon Sep 17 00:00:00 2001
From: "Brian S. O'Neill" <bronee@gmail.com>
Date: Wed, 15 Nov 2006 20:45:06 +0000
Subject: More work to avoid race conditions and deadlock.

---
 .../amazon/carbonado/spi/StorageCollection.java    | 98 ++++++++++++----------
 1 file changed, 53 insertions(+), 45 deletions(-)

diff --git a/src/main/java/com/amazon/carbonado/spi/StorageCollection.java b/src/main/java/com/amazon/carbonado/spi/StorageCollection.java
index 45ab6f5..c75f350 100644
--- a/src/main/java/com/amazon/carbonado/spi/StorageCollection.java
+++ b/src/main/java/com/amazon/carbonado/spi/StorageCollection.java
@@ -19,8 +19,8 @@
 package com.amazon.carbonado.spi;
 
 import java.util.IdentityHashMap;
-import java.util.Map;
 
+import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.ConcurrentHashMap;
 
 import com.amazon.carbonado.MalformedTypeException;
@@ -40,12 +40,12 @@ import com.amazon.carbonado.info.StorableIntrospector;
  * @author Brian S O'Neill
  */
 public abstract class StorageCollection {
-    private final Map<Class<?>, Storage> mStorageMap;
-    private final Map<Class<?>, Object> mStorableTypeLockMap;
+    private final ConcurrentMap<Class<?>, Storage> mStorageMap;
+    private final ConcurrentMap<Class<?>, Object> mStorableTypeLockMap;
 
     public StorageCollection() {
         mStorageMap = new ConcurrentHashMap<Class<?>, Storage>();
-        mStorableTypeLockMap = new IdentityHashMap<Class<?>, Object>();
+        mStorableTypeLockMap = new ConcurrentHashMap<Class<?>, Object>();
     }
 
     public <S extends Storable> Storage<S> storageFor(Class<S> type)
@@ -56,55 +56,63 @@ public abstract class StorageCollection {
             return storage;
         }
 
-        Object lock;
-        boolean doCreate;
-
-        synchronized (mStorableTypeLockMap) {
-            lock = mStorableTypeLockMap.get(type);
-            if (lock != null) {
-                doCreate = false;
-            } else {
-                doCreate = true;
-                lock = new Object();
-                mStorableTypeLockMap.put(type, lock);
+        getLock: while (true) {
+            Object lock;
+            boolean doCreate;
+
+            {
+                Object newLock = new Object();
+                Object existingLock = mStorableTypeLockMap.putIfAbsent(type, newLock);
+
+                if (existingLock == null) {
+                    lock = newLock;
+                    doCreate = true;
+                } else {
+                    lock = existingLock;
+                    doCreate = false;
+                }
             }
-        }
 
-        if (Thread.holdsLock(lock)) {
-            throw new IllegalStateException
-                ("Recursively trying to create storage for type: " + type);
-        }
+            if (Thread.holdsLock(lock)) {
+                throw new IllegalStateException
+                    ("Recursively trying to create storage for type: " + type);
+            }
 
-        try {
             synchronized (lock) {
-                // Check storage map again before creating new storage.
-                while (true) {
-                    storage = mStorageMap.get(type);
-                    if (storage != null) {
-                        return storage;
+                try {
+                    // Check storage map again before creating new storage.
+                    checkLock: while (true) {
+                        storage = mStorageMap.get(type);
+                        if (storage != null) {
+                            return storage;
+                        }
+                        if (mStorableTypeLockMap.get(type) != lock) {
+                            // Lock has changed, so get a new lock.
+                            continue getLock;
+                        }
+                        if (doCreate) {
+                            break checkLock;
+                        }
+                        try {
+                            // Wait with a timeout to handle rare race condition.
+                            lock.wait(100);
+                        } catch (InterruptedException e) {
+                            throw new RepositoryException("Interrupted");
+                        }
                     }
-                    if (doCreate) {
-                        break;
-                    }
-                    try {
-                        lock.wait();
-                    } catch (InterruptedException e) {
-                        throw new RepositoryException("Interrupted");
-                    }
-                }
 
-                // Examine and throw exception early if there is a problem.
-                StorableIntrospector.examine(type);
+                    // Examine and throw exception early if there is a problem.
+                    StorableIntrospector.examine(type);
 
-                storage = createStorage(type);
+                    storage = createStorage(type);
 
-                mStorageMap.put(type, storage);
-                lock.notifyAll();
-            }
-        } finally {
-            // Storable type lock no longer needed.
-            synchronized (mStorableTypeLockMap) {
-                mStorableTypeLockMap.remove(type);
+                    mStorageMap.put(type, storage);
+                    break getLock;
+                } finally {
+                    // Storable type lock no longer needed.
+                    mStorableTypeLockMap.remove(type, lock);
+                    lock.notifyAll();
+                }
             }
         }
 
-- 
cgit v1.2.3