[uima-uimaj] 01/01: [UIMA-6413] Memory leak in FSClassRegistry

[re...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

rec pushed a commit to branch bugfix/UIMA-6413-Memory-leak-in-FSClassRegistry
in repository https://gitbox.apache.org/repos/asf/uima-uimaj.git

commit 410a8e95ce6e8107a343c7972d9b04ef262d1158
Author: Richard Eckart de Castilho <re...@apache.org>
AuthorDate: Fri Jan 21 10:33:27 2022 +0100

    [UIMA-6413] Memory leak in FSClassRegistry
    
    - Unregister UIMAClassLoader form the FSClassRegistry when close() is called
---
 .../org/apache/uima/cas/impl/FSClassRegistry.java  | 15 ++++-
 .../apache/uima/internal/util/UIMAClassLoader.java |  4 ++
 .../apache/uima/cas/impl/FSClassRegistryTest.java  | 76 ++++++++++++++++++++++
 3 files changed, 94 insertions(+), 1 deletion(-)

diff --git a/uimaj-core/src/main/java/org/apache/uima/cas/impl/FSClassRegistry.java b/uimaj-core/src/main/java/org/apache/uima/cas/impl/FSClassRegistry.java
index 439793e..118db1b 100644
--- a/uimaj-core/src/main/java/org/apache/uima/cas/impl/FSClassRegistry.java
+++ b/uimaj-core/src/main/java/org/apache/uima/cas/impl/FSClassRegistry.java
@@ -299,6 +299,10 @@ public abstract class FSClassRegistry { // abstract to prevent instantiating; th
     reportErrors();
   }
 
+  static int clToType2JCasSize() {
+    return cl_to_type2JCas.size();
+  }
+
   private static void loadBuiltins(TypeImpl ti, ClassLoader cl, Map<String, JCasClassInfo> type2jcci, ArrayList<MutableCallSite> callSites_toSync) {
     String typeName = ti.getName();
     
@@ -1383,7 +1387,7 @@ public abstract class FSClassRegistry { // abstract to prevent instantiating; th
     for (FsGenerator3 v : r) {
       if (v != null)
         return false;
-    }
+      }
     return true;
   }
 
@@ -1465,6 +1469,15 @@ public abstract class FSClassRegistry { // abstract to prevent instantiating; th
     }
   }
   
+  /**
+   * For internal use only!
+   */
+  public static void unregister_jcci_classloader(ClassLoader cl) {
+    synchronized (cl_to_type2JCas) {
+      cl_to_type2JCas.remove(cl);
+    }
+  }
+
   static Map<String, JCasClassInfo> get_className_to_jcci(ClassLoader cl, boolean is_pear) {
     synchronized (cl_to_type2JCas) {
       // This was used before switching from the normal synchronized map to the weak map
diff --git a/uimaj-core/src/main/java/org/apache/uima/internal/util/UIMAClassLoader.java b/uimaj-core/src/main/java/org/apache/uima/internal/util/UIMAClassLoader.java
index 84d32c0..9f9f2dc 100644
--- a/uimaj-core/src/main/java/org/apache/uima/internal/util/UIMAClassLoader.java
+++ b/uimaj-core/src/main/java/org/apache/uima/internal/util/UIMAClassLoader.java
@@ -28,6 +28,8 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.StringTokenizer;
 
+import org.apache.uima.cas.impl.FSClassRegistry;
+
 /**
  * UIMAClassLoader is used as extension ClassLoader for UIMA to load additional components like
  * annotators and resources. The classpath of the classloader is specified as string.
@@ -202,6 +204,7 @@ public class UIMAClassLoader extends URLClassLoader {
    * Try to load the class itself before delegate the class loading to its parent
    * String is like x.y.Foo
    */
+  @Override
   protected Class<?> loadClass(String name, boolean resolve)
           throws ClassNotFoundException {
  
@@ -269,6 +272,7 @@ public class UIMAClassLoader extends URLClassLoader {
   @Override
   public void close() throws IOException {
     isClosed = true;
+    FSClassRegistry.unregister_jcci_classloader(this);
     super.close();
   }
   
diff --git a/uimaj-core/src/test/java/org/apache/uima/cas/impl/FSClassRegistryTest.java b/uimaj-core/src/test/java/org/apache/uima/cas/impl/FSClassRegistryTest.java
new file mode 100644
index 0000000..d05bcde
--- /dev/null
+++ b/uimaj-core/src/test/java/org/apache/uima/cas/impl/FSClassRegistryTest.java
@@ -0,0 +1,76 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.uima.cas.impl;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.apache.uima.UIMAFramework;
+import org.apache.uima.jcas.JCas;
+import org.apache.uima.resource.ResourceManager;
+import org.apache.uima.util.CasCreationUtils;
+import org.junit.Test;
+
+public class FSClassRegistryTest {
+  @Test
+  public void thatCreatingResourceManagersWithExtensionClassloaderDoesNotFillUpCache()
+          throws Exception {
+    for (int i = 0; i < 5; i++) {
+      ResourceManager resMgr = UIMAFramework.newDefaultResourceManager();
+      resMgr.setExtensionClassLoader(getClass().getClassLoader(), true);
+      JCas jcas = CasCreationUtils.createCas(null, null, null, resMgr).getJCas();
+
+      ClassLoader cl = jcas.getCasImpl().getJCasClassLoader();
+      assertThat(cl.getResource(FSClassRegistryTest.class.getName().replace(".", "/") + ".class")) //
+              .isNotNull();
+
+      assertThat(FSClassRegistry.clToType2JCasSize()) //
+              .as("System classloader + UIMAClassLoader") //
+              .isEqualTo(2);
+
+      resMgr.destroy();
+
+      assertThat(FSClassRegistry.clToType2JCasSize()) //
+              .as("System classloader only") //
+              .isEqualTo(1);
+    }
+  }
+
+  @Test
+  public void thatCreatingResourceManagersWithExtensionPathDoesNotFillUpCache() throws Exception {
+    for (int i = 0; i < 5; i++) {
+      ResourceManager resMgr = UIMAFramework.newDefaultResourceManager();
+      resMgr.setExtensionClassPath("src/test/java", true);
+      JCas jcas = CasCreationUtils.createCas(null, null, null, resMgr).getJCas();
+
+      ClassLoader cl = jcas.getCasImpl().getJCasClassLoader();
+      assertThat(cl.getResource(FSClassRegistryTest.class.getName().replace(".", "/") + ".java")) //
+              .isNotNull();
+
+      assertThat(FSClassRegistry.clToType2JCasSize()) //
+              .as("System classloader + UIMAClassLoader") //
+              .isEqualTo(2);
+
+      resMgr.destroy();
+
+      assertThat(FSClassRegistry.clToType2JCasSize()) //
+              .as("System classloader only") //
+              .isEqualTo(1);
+    }
+  }
+}