JDBC-364 Incorrect use of WeakHashMap in FBManagedConnectionFactory a…

…nd FBDriver

src/jdbc_40/org/firebirdsql/jdbc/FBDriver.java

@@ -1,7 +1,7 @@
 /*
  * $Id$
- * 
- * Firebird Open Source J2ee connector - jdbc driver
+ *
+ * Firebird Open Source JavaEE Connector - JDBC Driver
  *
  * Distributable under LGPL license.
  * You may obtain a copy of the License at http://www.gnu.org/copyleft/lgpl.html
@@ -14,13 +14,12 @@
  * This file was created by members of the firebird development team.
  * All individual contributions remain the Copyright (C) of those
  * individuals.  Contributors to this file are either listed here or
- * can be obtained from a CVS history command.
+ * can be obtained from a source control history command.
  *
  * All rights reserved.
  */
 package org.firebirdsql.jdbc;
 
-
 import java.sql.*;
 import java.util.logging.Logger;
 

src/main/org/firebirdsql/jca/FBManagedConnectionFactory.java

@@ -1,7 +1,7 @@
 /*
  * $Id$
- * 
- * Firebird Open Source J2ee connector - jdbc driver
+ *
+ * Firebird Open Source JavaEE Connector - JDBC Driver
  *
  * Distributable under LGPL license.
  * You may obtain a copy of the License at http://www.gnu.org/copyleft/lgpl.html
@@ -14,18 +14,21 @@
  * This file was created by members of the firebird development team.
  * All individual contributions remain the Copyright (C) of those
  * individuals.  Contributors to this file are either listed here or
- * can be obtained from a CVS history command.
+ * can be obtained from a source control history command.
  *
  * All rights reserved.
  */
-
 package org.firebirdsql.jca;
 
 import java.io.*;
+import java.lang.ref.Reference;
+import java.lang.ref.ReferenceQueue;
+import java.lang.ref.SoftReference;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
 import java.sql.SQLException;
 import java.util.*;
+import java.util.concurrent.ConcurrentHashMap;
 
 import javax.resource.NotSupportedException;
 import javax.resource.ResourceException;
@@ -61,8 +64,14 @@ public class FBManagedConnectionFactory implements ManagedConnectionFactory,
     /**
      * The <code>mcfInstances</code> weak hash map is used in deserialization
      * to find the correct instance of a mcf after deserializing.
+     * <p>
+     * It is also used to return a canonical instance to {@link org.firebirdsql.jdbc.FBDriver}.
+     * </p>
      */
-    private final static Map mcfInstances = Collections.synchronizedMap(new WeakHashMap());
+    private static final Map<FBConnectionProperties, SoftReference<FBManagedConnectionFactory>> mcfInstances =
+            new ConcurrentHashMap<FBConnectionProperties, SoftReference<FBManagedConnectionFactory>>();
+    private static final ReferenceQueue<FBManagedConnectionFactory> mcfReferenceQueue =
+            new ReferenceQueue<FBManagedConnectionFactory>();
 
     // /**
     // * @todo Claudio suggests this should be 1024*64 -1, we should find out I
@@ -584,14 +593,10 @@ public PrintWriter getLogWriter() {
 
     // Serialization support
     private Object readResolve() throws ObjectStreamException {
-        FBManagedConnectionFactory mcf = 
-            (FBManagedConnectionFactory) mcfInstances.get(this);
-
-        if (mcf != null)  return mcf; 
-
-        mcf = new FBManagedConnectionFactory(getGDSType(), 
-                (FBConnectionProperties)this.connectionProperties.clone());
-
+        FBManagedConnectionFactory mcf = internalCanonicalize();
+        if (mcf != null)  return mcf;
+
+        mcf = new FBManagedConnectionFactory(getGDSType(), (FBConnectionProperties)this.connectionProperties.clone());
         return mcf;
     }
 
@@ -603,20 +608,40 @@ private Object readResolve() throws ObjectStreamException {
      * @return a <code>FBManagedConnectionFactory</code> value
      */
     public FBManagedConnectionFactory canonicalize() {
-        FBManagedConnectionFactory mcf = (FBManagedConnectionFactory) mcfInstances.get(this);
-
-        if (mcf != null) 
-            return mcf;
-
+        final FBManagedConnectionFactory mcf = internalCanonicalize();
+        if (mcf != null) return mcf;
+        start();
         return this;
     }
 
+    private FBManagedConnectionFactory internalCanonicalize() {
+        final SoftReference<FBManagedConnectionFactory> factoryReference = mcfInstances.get(getCacheKey());
+        return factoryReference != null ? factoryReference.get() : null;
+    }
+
+    /**
+     * Starts this MCF and adds this instance to {@code #mcfInstances} cache.
+     * <p>
+     * This implementation (together with {@link #canonicalize()} has a race condition with regard to the
+     * instance cache. As this is a relatively harmless one, we leave it as is.
+     * </p>
+     */
     private void start() {
         synchronized (startLock) {
-            if (!started) {
-                mcfInstances.put(this, this);
-                started = true;
-            } // end of if ()
+            if (started) return;
+            mcfInstances.put(getCacheKey(), new SoftReference<FBManagedConnectionFactory>(this, mcfReferenceQueue));
+            started = true;
+        }
+        cleanMcfInstances();
+    }
+
+    /**
+     * Removes cleared references from the {@link #mcfInstances} cache.
+     */
+    private void cleanMcfInstances() {
+        Reference<? extends FBManagedConnectionFactory> reference;
+        while ((reference = mcfReferenceQueue.poll()) != null) {
+            mcfInstances.values().remove(reference);
         }
     }
 
@@ -678,8 +703,8 @@ public void recover(FBManagedConnection mc, Xid xid) throws GDSException {
      * rollback. If no "in limbo" transaction can be found, or error happens
      * during completion, an exception is thrown.
      * 
-     * @param gdsHelper
-     *            instance of {@link GDSHelper} that will be used to reconnect
+     * @param gds
+     *            instance of {@link GDS} that will be used to reconnect
      *            transaction.
      * @param xid
      *            Xid of the transaction to reconnect.
@@ -832,4 +857,8 @@ AbstractConnection newConnection(FBManagedConnection mc)
                     + connectionClass.getName());
         }
     }
+
+    public FBConnectionProperties getCacheKey() {
+        return (FBConnectionProperties) connectionProperties.clone();
+    }
 }

src/main/org/firebirdsql/jdbc/AbstractDriver.java

@@ -1,5 +1,7 @@
 /*
- * Firebird Open Source J2ee connector - jdbc driver
+ * $Id$
+ *
+ * Firebird Open Source JavaEE Connector - JDBC Driver
  *
  * Distributable under LGPL license.
  * You may obtain a copy of the License at http://www.gnu.org/copyleft/lgpl.html
@@ -12,15 +14,18 @@
  * This file was created by members of the firebird development team.
  * All individual contributions remain the Copyright (C) of those
  * individuals.  Contributors to this file are either listed here or
- * can be obtained from a CVS history command.
+ * can be obtained from a source control history command.
  *
  * All rights reserved.
  */
 package org.firebirdsql.jdbc;
 
-
+import java.lang.ref.Reference;
+import java.lang.ref.ReferenceQueue;
+import java.lang.ref.SoftReference;
 import java.sql.*;
 import java.util.*;
+import java.util.concurrent.ConcurrentHashMap;
 
 import javax.resource.ResourceException;
 
@@ -54,7 +59,10 @@ public abstract class AbstractDriver implements FirebirdDriver {
      * standard connection.
      */
 
-    private Map mcfToDataSourceMap = Collections.synchronizedMap(new WeakHashMap());
+    private final Map<FBConnectionProperties, SoftReference<FBDataSource>> mcfToDataSourceMap =
+            new ConcurrentHashMap<FBConnectionProperties, SoftReference<FBDataSource>>();
+    private final ReferenceQueue<FBDataSource> dataSourceReferenceQueue = new ReferenceQueue<FBDataSource>();
+    private final Object createDataSourceLock = new Object();
 
     static {
         log = LoggerFactory.getLogger(AbstractDriver.class, false);
@@ -137,16 +145,37 @@ public Connection connect(String url, Properties originalInfo) throws SQLExcepti
     }
 
     private FBDataSource createDataSource(FBManagedConnectionFactory mcf) throws ResourceException {
-        FBDataSource dataSource = null;
-        synchronized (mcfToDataSourceMap) {
-            dataSource = (FBDataSource)mcfToDataSourceMap.get(mcf);
-
-            if (dataSource == null) {
-                dataSource = (FBDataSource)mcf.createConnectionFactory();
-                mcfToDataSourceMap.put(mcf, dataSource);
+        try {
+            FBDataSource dataSource = dataSourceFromCache(mcf);
+            if (dataSource != null) return dataSource;
+            synchronized (createDataSourceLock) {
+                // Obtain again
+                dataSource = dataSourceFromCache(mcf);
+                if (dataSource == null) {
+                    dataSource = (FBDataSource) mcf.createConnectionFactory();
+                    mcfToDataSourceMap.put(mcf.getCacheKey(),
+                            new SoftReference<FBDataSource>(dataSource, dataSourceReferenceQueue));
+                }
             }
+            return dataSource;
+        } finally {
+            cleanDataSourceCache();
         }
-        return dataSource;
+    }
+
+    /**
+     * Removes cleared references from the {@link #mcfToDataSourceMap} cache.
+     */
+    private void cleanDataSourceCache() {
+        Reference<? extends FBDataSource> reference;
+        while ((reference = dataSourceReferenceQueue.poll()) != null) {
+            mcfToDataSourceMap.values().remove(reference);
+        }
+    }
+
+    private FBDataSource dataSourceFromCache(FBManagedConnectionFactory mcf) {
+        final SoftReference<FBDataSource> dataSourceReference = mcfToDataSourceMap.get(mcf.getCacheKey());
+        return dataSourceReference != null ? dataSourceReference.get() : null;
     }
 
     public FirebirdConnection connect(FirebirdConnectionProperties properties) throws SQLException {