From c393329daffb101c058030f61a41aab11d6ca088 Mon Sep 17 00:00:00 2001 From: Ryan Lubke Date: Thu, 13 Jul 2023 05:04:37 +0000 Subject: [PATCH] BUG 35590953 - [35544351->23.03.2] Memory leak: After restarting View service, serviceMap size increases (merge ce/main -> ce/23.03 @ 101735) [git-p4: depot-paths = "//dev/coherence-ce/release/coherence-ce-v23.03/": change = 101736] --- .../coherence/config/scheme/ViewScheme.java | 108 +++++++++++++----- .../src/main/java/jmx/CacheMBeanTests.java | 19 +-- 2 files changed, 90 insertions(+), 37 deletions(-) diff --git a/prj/coherence-core/src/main/java/com/tangosol/coherence/config/scheme/ViewScheme.java b/prj/coherence-core/src/main/java/com/tangosol/coherence/config/scheme/ViewScheme.java index e24baa3e78f67..fd57838cd0cf1 100644 --- a/prj/coherence-core/src/main/java/com/tangosol/coherence/config/scheme/ViewScheme.java +++ b/prj/coherence-core/src/main/java/com/tangosol/coherence/config/scheme/ViewScheme.java @@ -6,8 +6,6 @@ */ package com.tangosol.coherence.config.scheme; -import com.oracle.coherence.common.collections.ConcurrentHashMap; - import com.tangosol.config.expression.ParameterResolver; import com.tangosol.internal.net.service.DefaultViewDependencies; @@ -21,10 +19,12 @@ import com.tangosol.net.cache.ContinuousQueryCache; +import com.tangosol.net.internal.ScopedServiceReferenceStore; import com.tangosol.net.internal.ViewCacheService; -import com.tangosol.util.RegistrationBehavior; -import com.tangosol.util.ResourceRegistry; +import com.tangosol.util.Base; + +import java.lang.reflect.Method; /** * A Scheme that realizes both services and caches for Coherence 12.2.1.4 feature @@ -41,10 +41,41 @@ // caches are primed eagerly with data to have similar behavior semantics as // replicated caches. Unlike regular services this service does not have its // own type and is *not* instantiated or maintained by the cluster 'consciously' -// (we use the cluster's resource registry to store these services). +// (we use the cluster's ScopedServiceReferenceStore to store these services). +@SuppressWarnings("rawtypes") public class ViewScheme extends AbstractCompositeScheme { + + // Internal Notes: + // Static initialization to obtain and store a reference + // to SafeCluster.getScopedServiceStore(). This store will be used to + // store ViewCacheService instances. + // + // Originally, these instances were stored in the Cluster's resource + // registry, however, the registry contents won't survive a cluster + // stop/start (like a member eviction/restart). + // The ScopedServiceReferenceStore will survive in this scenario + // allowing the same ViewCacheService instance to be used and thus + // avoiding a potential memory leak. + static + { + Class clzSafeCluster; + try + { + clzSafeCluster = Class.forName("com.tangosol.coherence.component.util.SafeCluster", + false, + ViewScheme.class.getClassLoader()); + SAFE_CLUSTER_GET_SCOPED_SERVICE_STORE = + clzSafeCluster.getDeclaredMethod("getScopedServiceStore"); + + } + catch (Exception e) + { + throw Base.ensureRuntimeException(e, "ViewScheme initialization failed"); + } + } + // ----- constructors --------------------------------------------------- /** @@ -61,6 +92,14 @@ public ViewScheme() setBackScheme(schemeBack); } + // ----- AbstractServiceScheme methods ---------------------------------- + + @Override + public String getScopedServiceName() + { + return ViewCacheService.KEY_CLUSTER_REGISTRY + '-' + super.getScopedServiceName(); + } + // ----- ServiceBuilder interface --------------------------------------- @Override @@ -69,25 +108,9 @@ public Service realizeService(ParameterResolver resolver, ClassLoader loader, Cl validate(); // see if we've already created the appropriate CacheService - ResourceRegistry registry = cluster.getResourceRegistry(); - - ConcurrentHashMap mapServices = - registry.getResource(ConcurrentHashMap.class, ViewCacheService.KEY_CLUSTER_REGISTRY); - - if (mapServices == null) - { - registry.registerResource( - ConcurrentHashMap.class, - ViewCacheService.KEY_CLUSTER_REGISTRY, - ConcurrentHashMap::new, - RegistrationBehavior.IGNORE, - null); - - mapServices = registry.getResource(ConcurrentHashMap.class, ViewCacheService.KEY_CLUSTER_REGISTRY); - } - - String sServiceName = getScopedServiceName(); - Service service = mapServices.get(sServiceName); + ScopedServiceReferenceStore store = getServiceStore(cluster); + String sServiceName = getScopedServiceName(); + Service service = store.getService(sServiceName); if (service != null) { return service; @@ -110,10 +133,7 @@ public Service realizeService(ParameterResolver resolver, ClassLoader loader, Cl service = new ViewCacheService(serviceBack); service.setDependencies(deps); - if (mapServices.putIfAbsent(sServiceName, service) != null) - { - service = mapServices.get(sServiceName); // highly unlikely path - } + store.putService(service, sServiceName, getServiceType()); return service; } @@ -133,10 +153,42 @@ public String getServiceType() return ViewCacheService.TYPE_VIEW; } + // ----- helper methods ------------------------------------------------- + + /** + * Obtain the {@link ScopedServiceReferenceStore} from the provided + * {@link Cluster}. + * + * @param cluster the {@link Cluster} + * + * @return the {@link ScopedServiceReferenceStore} from the provided + * {@link Cluster}. + * + * @since 12.2.1.4.19 + */ + protected ScopedServiceReferenceStore getServiceStore(Cluster cluster) + { + try + { + return (ScopedServiceReferenceStore) SAFE_CLUSTER_GET_SCOPED_SERVICE_STORE.invoke(cluster); + } + catch (Exception e) + { + throw Base.ensureRuntimeException(e, "Failed to invoke SafeCluster.getScopedReferenceStore()"); + } + } + // ----- constants ------------------------------------------------------ /** * A CachingScheme that represents NO_VALUE. */ private static final CachingScheme NO_SCHEME = new ClassScheme(); + + /** + * Method reference to obtain SafeCluster ScopedServiceReferenceStore. + * + * @since 12.2.1.4.19 + */ + private static final Method SAFE_CLUSTER_GET_SCOPED_SERVICE_STORE; } diff --git a/prj/test/functional/jmx/src/main/java/jmx/CacheMBeanTests.java b/prj/test/functional/jmx/src/main/java/jmx/CacheMBeanTests.java index 2756c997233cb..20ef949bbeafa 100644 --- a/prj/test/functional/jmx/src/main/java/jmx/CacheMBeanTests.java +++ b/prj/test/functional/jmx/src/main/java/jmx/CacheMBeanTests.java @@ -1,17 +1,20 @@ /* - * Copyright (c) 2000, 2022, Oracle and/or its affiliates. + * Copyright (c) 2000, 2023, Oracle and/or its affiliates. * * Licensed under the Universal Permissive License v 1.0 as shown at * https://oss.oracle.com/licenses/upl. */ package jmx; -import com.oracle.coherence.common.collections.ConcurrentHashMap; +import com.tangosol.coherence.component.util.SafeCluster; import com.tangosol.net.CacheFactory; import com.tangosol.net.NamedCache; import com.tangosol.net.Service; + +import com.tangosol.net.internal.ScopedServiceReferenceStore; import com.tangosol.net.internal.ViewCacheService; + import com.tangosol.net.management.MBeanHelper; import com.tangosol.net.management.Registry; @@ -268,7 +271,7 @@ protected void validateViewMBean(String sCacheName, int cCacheSize) + ",nodeId=1,service=" + cache.getCacheService().getInfo().getServiceName() + "," + Registry.VIEW_TYPE; - ObjectName nameMBean = new ObjectName(sName); + ObjectName nameMBean = new ObjectName(sName); Set setObjectNames = serverJMX.queryNames(nameMBean, null); assertEquals(1, setObjectNames.size()); @@ -282,11 +285,9 @@ protected void validateViewMBean(String sCacheName, int cCacheSize) } - ConcurrentHashMap mapServices = cache.getService() - .getCluster() - .getResourceRegistry() - .getResource(ConcurrentHashMap.class, ViewCacheService.KEY_CLUSTER_REGISTRY); - Service service = mapServices.get("DistributedCache"); + SafeCluster safeCluster = (SafeCluster) cache.getService().getCluster(); + ScopedServiceReferenceStore store = safeCluster.getScopedServiceStore(); + Service service = store.getService(ViewCacheService.KEY_CLUSTER_REGISTRY + "-DistributedCache"); ((ViewCacheService)service).destroyCache(cache); setObjectNames = serverJMX.queryNames(nameMBean, null); @@ -336,4 +337,4 @@ protected ObjectName getQueryName(NamedCache cache, boolean fFront) * cache configuration file with the required schemes. */ public final static String FILE_CFG_CACHE = "cache-config.xml"; - } \ No newline at end of file + }