Skip to content

Embed URL

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Proposed fix for GLASSFISH-19746 and GLASSFISH-19508 based on SVN revision 55564 branch 3.1.2.2-no-delete
# 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);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.