[uima-uimaj] branch bugfix/UIMA-6413-Memory-leak-in-FSClassRegistry created (now 4811860)

[re...@apache.org]

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

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


      at 4811860  [UIMA-6413] Memory leak in FSClassRegistry

This branch includes the following new commits:

     new 4811860  [UIMA-6413] Memory leak in FSClassRegistry

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.

[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 481186098bfb368e2db510bfd29f718c47234c42
Author: Richard Eckart de Castilho <re...@apache.org>
AuthorDate: Thu Jan 27 08:05:00 2022 +0100

    [UIMA-6413] Memory leak in FSClassRegistry
    
    - Handle the circular dependency between the static initializer blocks of TypeSystemImpl and FSClassRegistry in UIMAClassLoader and in the FSClassRegistryTest
    - Add another simple test that just creates a resource manager with an extension classpath and then destroys it again to ensure it does not generate an exception when trying to unregister the resource manager's UIMAClassLoader from the FSClassRegistry on destroy
---
 .../org/apache/uima/internal/util/UIMAClassLoader.java     | 12 +++++++++++-
 .../java/org/apache/uima/cas/impl/FSClassRegistryTest.java | 11 +++++++----
 .../uima/resource/impl/ResourceManager_implTest.java       | 14 +++++++++++++-
 3 files changed, 31 insertions(+), 6 deletions(-)

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 9f9f2dc..ccfaec5 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
@@ -29,6 +29,7 @@ import java.util.List;
 import java.util.StringTokenizer;
 
 import org.apache.uima.cas.impl.FSClassRegistry;
+import org.apache.uima.cas.impl.TypeSystemImpl;
 
 /**
  * UIMAClassLoader is used as extension ClassLoader for UIMA to load additional components like
@@ -272,7 +273,16 @@ public class UIMAClassLoader extends URLClassLoader {
   @Override
   public void close() throws IOException {
     isClosed = true;
-    FSClassRegistry.unregister_jcci_classloader(this);
+    // There is a circular dependency between the static initializer blocks of FSClassRegistry and
+    // TypeSystemImpl which requires that the TypeSystemImpl class must be initialized before the
+    // FSClassRegistry to avoid exceptions. The if-statement here is a red-herring because the
+    // actual comparison does not really matter - under normal circumstances, `staticTsi` cannot be
+    // null.
+    // However, what it really does is trigger the static initialization block of TypeSystemImpl
+    // so that the subsequent call to FSClassRegistry does not trigger an exception.
+    if (TypeSystemImpl.staticTsi != null) {
+      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
index 3f04259..78379f9 100644
--- 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
@@ -32,13 +32,18 @@ public class FSClassRegistryTest {
   @Before
   public void setup() {
     System.setProperty(FSClassRegistry.RECORD_JCAS_CLASSLOADERS, "true");
+
+    // Calls to FSClassRegistry will fail unless the static initializer block in TypeSystemImpl
+    // has previously been triggered! During normal UIMA operations, this should not happen,
+    // in particular because FSClassRegistry is not really part of the public UIMA API -
+    // but in the minimal setup here, we need to make sure TypeSystemImpl has been initialized
+    // first.
+    new TypeSystemImpl();
   }
 
   @Test
   public void thatCreatingResourceManagersWithExtensionClassloaderDoesNotFillUpCache()
           throws Exception {
-    // Needed to get the type system code initialized before we call clToType2JCasSize();
-    CasCreationUtils.createCas();
     int numberOfCachedClassloadersAtStart = FSClassRegistry.clToType2JCasSize();
     for (int i = 0; i < 5; i++) {
       ResourceManager resMgr = UIMAFramework.newDefaultResourceManager();
@@ -60,8 +65,6 @@ public class FSClassRegistryTest {
 
   @Test
   public void thatCreatingResourceManagersWithExtensionPathDoesNotFillUpCache() throws Exception {
-    // Needed to get the type system code initialized before we call clToType2JCasSize();
-    CasCreationUtils.createCas();
     int numberOfCachedClassloadersAtStart = FSClassRegistry.clToType2JCasSize();
     for (int i = 0; i < 5; i++) {
       ResourceManager resMgr = UIMAFramework.newDefaultResourceManager();
diff --git a/uimaj-core/src/test/java/org/apache/uima/resource/impl/ResourceManager_implTest.java b/uimaj-core/src/test/java/org/apache/uima/resource/impl/ResourceManager_implTest.java
index fd5b8a8..91a443b 100644
--- a/uimaj-core/src/test/java/org/apache/uima/resource/impl/ResourceManager_implTest.java
+++ b/uimaj-core/src/test/java/org/apache/uima/resource/impl/ResourceManager_implTest.java
@@ -19,6 +19,8 @@
 
 package org.apache.uima.resource.impl;
 
+import static org.assertj.core.api.Assertions.assertThatCode;
+
 import java.io.File;
 import java.io.IOException;
 import java.net.URI;
@@ -37,8 +39,8 @@ import org.apache.uima.resource.metadata.impl.ExternalResourceBinding_impl;
 import org.apache.uima.resource.metadata.impl.ResourceManagerConfiguration_impl;
 import org.apache.uima.test.junit_extension.JUnitExtension;
 import org.apache.uima.util.XMLInputSource;
-
 import org.junit.Assert;
+
 import junit.framework.TestCase;
 
 
@@ -71,6 +73,7 @@ public class ResourceManager_implTest extends TestCase {
   /**
    * @see junit.framework.TestCase#setUp()
    */
+  @Override
   protected void setUp() throws Exception {
     try {
       super.setUp();
@@ -155,6 +158,7 @@ public class ResourceManager_implTest extends TestCase {
     }
   }
 
+  @Override
   public void tearDown() {
     mManager = null;
   }
@@ -271,4 +275,12 @@ public class ResourceManager_implTest extends TestCase {
       JUnitExtension.handleException(e);
     }
   }
+
+  public void testCreateWithExtensionClassloaderAndDestroy() throws Exception {
+    assertThatCode(() -> {
+      ResourceManager resMgr = UIMAFramework.newDefaultResourceManager();
+      resMgr.setExtensionClassLoader(getClass().getClassLoader(), false);
+      resMgr.destroy();
+    }).doesNotThrowAnyException();
+  }
 }