Proposed fix for GLASSFISH-19746 and GLASSFISH-19508 based on SVN revision 55564 branch 3.1.2.2-no-delete

gistfile1.diff

# HG changeset patch
# User Patrik Dudits <patrik@dudits.net>
# Date 1362140263 -3600
# Branch original
# Node ID 9820e648fc3b86f6b119254d7ceadd783a3168bb
# Parent  b117e6807cf56ef428781b453d1c20b182084ebe
GLASSFISH-19746: Memory leak in dynamic reconfiguration
- Instead of relying on Map of all created proxies that are only cleared during
  JDBC resource redeployment, use version counter.

diff -r b117e6807cf5 -r 9820e648fc3b connectors/connectors-runtime/src/main/java/com/sun/enterprise/connectors/ConnectorRegistry.java
--- a/connectors/connectors-runtime/src/main/java/com/sun/enterprise/connectors/ConnectorRegistry.java  Thu Jan 10 15:12:49 2013 +0100
+++ b/connectors/connectors-runtime/src/main/java/com/sun/enterprise/connectors/ConnectorRegistry.java	Fri Mar 01 13:17:43 2013 +0100
@@ -53,6 +53,7 @@
 import javax.resource.spi.ManagedConnectionFactory;
 import javax.validation.Validator;
 import java.util.*;
+import java.util.concurrent.atomic.AtomicLong;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -86,7 +87,7 @@
     protected final Map<String, ResourceAdapterConfig> resourceAdapterConfig;
     protected final Map<String, ConnectorApplication> rarModules;
     protected final Map<String, Validator> beanValidators;
-    protected final Map<ResourceInfo, Map<DynamicallyReconfigurableResource, Boolean>> resourceProxies;
+    protected final ConcurrentMap<ResourceInfo, AtomicLong> resourceInfoVersion;
     protected final Set<ResourceInfo> resourceInfos;
     protected final Set<PoolInfo> transparentDynamicReconfigPools;
     protected final Map<String, Object> locks;
@@ -114,7 +115,7 @@
         resourceAdapterConfig = Collections.synchronizedMap(new HashMap<String, ResourceAdapterConfig>());
         rarModules = Collections.synchronizedMap(new HashMap<String, ConnectorApplication>());
         beanValidators = Collections.synchronizedMap(new HashMap<String, Validator>());
-        resourceProxies = new HashMap<ResourceInfo, Map<DynamicallyReconfigurableResource, Boolean>>();
+        resourceInfoVersion = new ConcurrentHashMap<ResourceInfo, AtomicLong>();
         resourceInfos = new HashSet<ResourceInfo>();
         transparentDynamicReconfigPools = new HashSet<PoolInfo>();
         locks = new HashMap<String, Object>();
@@ -142,36 +143,45 @@
     }
 
     /**
-     * get the map of factories (proxy to actual factory) using the resource. 
+     * get the version counter of  a resource info
      * @param resourceInfo resource-name
-     * @return Map of factories
+     * @return version counter. {@code -1L} if the resource is invalid
+     */    
+    public long getResourceInfoVersion(ResourceInfo resourceInfo) {
+        AtomicLong version = resourceInfoVersion.get(resourceInfo);
+        if (version == null) {
+            // resource is no longer valid
+            return -1L;
+        } else {
+            return version.get();
+        }
+    }
+   
+    /**
+     * Update version information for a resource.
+     * @param resourceInfo resource info to be updated.
+     * @return new version couter
      */
-    public Map<DynamicallyReconfigurableResource, Boolean> getResourceFactories(ResourceInfo resourceInfo){
-        Map<DynamicallyReconfigurableResource, Boolean> map = resourceProxies.get(resourceInfo);
-        //TODO synchronization
-        if(map == null){
-            map = new HashMap<DynamicallyReconfigurableResource, Boolean>();
-            resourceProxies.put(resourceInfo, map);
+    public long updateResourceInfoVersion(ResourceInfo resourceInfo) {
+        AtomicLong version = resourceInfoVersion.get(resourceInfo);
+        if (version == null) {
+            AtomicLong newVersion = new AtomicLong();
+            version = resourceInfoVersion.putIfAbsent(resourceInfo, newVersion);
+            if (version == null) {
+                version = newVersion;
+            }
         }
-        return map;
+        return version.incrementAndGet();
     }
-
+    
     /**
      * remove and invalidate factories (proxy to actual factory) using the resource.
      * @param resourceInfo resource-name
-     * @return boolean indicating whether the factories are invalidated or not
+     * @return boolean indicating whether the factories will get invalidated
      */
     public boolean removeResourceFactories(ResourceInfo resourceInfo){
-        boolean mapRemoved = false;
-        Map<DynamicallyReconfigurableResource, Boolean> map = resourceProxies.remove(resourceInfo);
-        if(map != null){
-            for(DynamicallyReconfigurableResource resource : map.keySet()){
-                resource.setInvalid();
-            }
-            map.clear();
-            mapRemoved = true;
-        }
-        return mapRemoved;
+        resourceInfoVersion.remove(resourceInfo);
+        return false; // we actually don't know if there are any resource factories instantiated.
     }
 
     /**
@@ -183,6 +193,7 @@
             synchronized (resourceInfos){
                 resourceInfos.add(resourceInfo);
             }
+            updateResourceInfoVersion(resourceInfo);
         }
     }
 
@@ -197,6 +208,7 @@
             synchronized (resourceInfos){
                 removed = resourceInfos.remove(resourceInfo);
             }
+            resourceInfoVersion.remove(resourceInfo);           
         }
         return removed;
     }
diff -r b117e6807cf5 -r 9820e648fc3b connectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/deployer/JdbcConnectionPoolDeployer.java
--- a/connectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/deployer/JdbcConnectionPoolDeployer.java	Thu Jan 10 15:12:49 2013 +0100
+++ b/connectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/deployer/JdbcConnectionPoolDeployer.java	Fri Mar 01 13:17:43 2013 +0100
@@ -911,15 +911,9 @@
         for (BindableResource bindableResource : resourcesList) {
 
             ResourceInfo resourceInfo = ConnectorsUtil.getResourceInfo(bindableResource);
-            Map<DynamicallyReconfigurableResource, Boolean> connectionFactories =
-                    ConnectorRegistry.getInstance().getResourceFactories(resourceInfo);
 
             debug("marking connection factories for refresh");
-
-            for (DynamicallyReconfigurableResource cf : connectionFactories.keySet()) {
-                debug("marking connection factory for refresh : " + cf);
-                connectionFactories.put(cf, Boolean.valueOf("false"));
-            }
+            ConnectorRegistry.getInstance().updateResourceInfoVersion(resourceInfo);
         }
     }
 
diff -r b117e6807cf5 -r 9820e648fc3b connectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/naming/ConnectorObjectFactory.java
--- a/connectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/naming/ConnectorObjectFactory.java	Thu Jan 10 15:12:49 2013 +0100
+++ b/connectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/naming/ConnectorObjectFactory.java	Fri Mar 01 13:17:43 2013 +0100
@@ -183,9 +183,6 @@
                                     proxyClasses[proxyClasses.length - 1] = DynamicallyReconfigurableResource.class;
 
                                     cf = getProxyObject(cf, proxyClasses, resourceInfo);
-                                    Map<DynamicallyReconfigurableResource, Boolean> resourceFactories =
-                                            ConnectorRegistry.getInstance().getResourceFactories(resourceInfo);
-                                    resourceFactories.put((DynamicallyReconfigurableResource) Proxy.getInvocationHandler(cf), true);
                                 }
                             }
                         }
diff -r b117e6807cf5 -r 9820e648fc3b connectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/naming/DynamicResourceReconfigurator.java
--- a/connectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/naming/DynamicResourceReconfigurator.java	Thu Jan 10 15:12:49 2013 +0100
+++ b/connectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/naming/DynamicResourceReconfigurator.java	Fri Mar 01 13:17:43 2013 +0100
@@ -70,6 +70,7 @@
     private Object actualObject;
     private ResourceInfo resourceInfo;
     private boolean invalid = false;
+    private long resourceInfoVersion = 0;
 
     protected final static Logger _logger = LogDomains.getLogger(DynamicResourceReconfigurator.class,LogDomains.RSR_LOGGER);
 
@@ -79,7 +80,7 @@
     }
 
     public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
-
+        
         if (invalid) {
             throw new ResourceException("Resource ["+ resourceInfo +"] instance is not valid any more");
         }
@@ -90,17 +91,22 @@
                 && args.length == 0) {
             setInvalid();
         } else {
-            Map<DynamicallyReconfigurableResource, Boolean> proxies =
-                    ConnectorRegistry.getInstance().getResourceFactories(resourceInfo);
-            Boolean status = proxies.get(this);
-            if (status == null || !status) {
-                debug("status is null or false: " + this);
+            long version = ConnectorRegistry.getInstance().getResourceInfoVersion(resourceInfo);
+            if (version == -1L) {
+                // since we're not keeping the list of proxies, we need to invalidate as soon we are aware of the fact
+                setInvalid();
+                invoke(proxy,method,args); // just to trigger the exception
+            }
+            
+            boolean status = resourceInfoVersion >= version;
+            resourceInfoVersion = version;
+            if (!status) {
+                debug("status is outdated: " + this);
                 Hashtable env = new Hashtable();
                 env.put(ConnectorConstants.DYNAMIC_RECONFIGURATION_PROXY_CALL, "TRUE");
                 //TODO ASR : resource-naming-service need to support "env" for module/app scope also
                 ResourceNamingService  namingService = ConnectorRuntime.getRuntime().getResourceNamingService();
                 actualObject = namingService.lookup(resourceInfo, resourceInfo.getName(), env);
-                proxies.put(this, true);
                 debug("actualObject : " + actualObject);
             }else{
                 debug("status is true: " + this);