You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ra...@apache.org on 2017/12/14 08:03:27 UTC

[sling-org-apache-sling-scripting-core] branch master updated: SLING-7311 - The SlingScriptEngineManager doesn't correctly flush script engine associations

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

radu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-scripting-core.git


The following commit(s) were added to refs/heads/master by this push:
     new a0bc18c  SLING-7311 - The SlingScriptEngineManager doesn't correctly flush script engine associations
a0bc18c is described below

commit a0bc18cc24dfe543b3214f2cd4ae63b362f63dcc
Author: Radu Cotescu <ra...@apache.org>
AuthorDate: Thu Dec 14 09:03:00 2017 +0100

    SLING-7311 - The SlingScriptEngineManager doesn't correctly flush script engine associations
    
    * added tests to verify stale entries
    * made sure the internal script engine manager is always refreshed on change
---
 .../core/impl/jsr223/SlingScriptEngineManager.java | 31 +++++++++-------------
 .../impl/jsr223/SlingScriptEngineManagerTest.java  | 25 ++++++++++++-----
 2 files changed, 32 insertions(+), 24 deletions(-)

diff --git a/src/main/java/org/apache/sling/scripting/core/impl/jsr223/SlingScriptEngineManager.java b/src/main/java/org/apache/sling/scripting/core/impl/jsr223/SlingScriptEngineManager.java
index 6f1ece9..f82bc04 100644
--- a/src/main/java/org/apache/sling/scripting/core/impl/jsr223/SlingScriptEngineManager.java
+++ b/src/main/java/org/apache/sling/scripting/core/impl/jsr223/SlingScriptEngineManager.java
@@ -18,12 +18,6 @@
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/
 package org.apache.sling.scripting.core.impl.jsr223;
 
-import java.io.BufferedReader;
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.InputStreamReader;
-import java.net.URL;
-import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Dictionary;
@@ -85,18 +79,7 @@ public class SlingScriptEngineManager extends ScriptEngineManager implements Bun
     private final Map<ScriptEngineFactory, Map<String, Object>> factoriesProperties = new HashMap<>();
     private final Set<ServiceReference<ScriptEngineFactory>> serviceReferences = new HashSet<>();
 
-    private final ScriptEngineManager internalManager;
-    {
-        ClassLoader loader = Thread.currentThread().getContextClassLoader();
-        try {
-            Thread.currentThread().setContextClassLoader(null);
-            internalManager = new ScriptEngineManager(ClassLoader.getSystemClassLoader());
-        }
-        finally {
-            Thread.currentThread().setContextClassLoader(loader);
-        }
-    }
-
+    private ScriptEngineManager internalManager;
     private SortedSet<SortableScriptEngineFactory> factories = new TreeSet<>();
     private ComponentContext componentContext;
 
@@ -236,6 +219,7 @@ public class SlingScriptEngineManager extends ScriptEngineManager implements Bun
     private void updateFactories() {
         readWriteLock.writeLock().lock();
         try {
+            internalManager = getInternalScriptEngineManager();
             factories = new TreeSet<>();
             long fakeBundleIdCounter = Long.MIN_VALUE;
             // first add the platform factories
@@ -289,6 +273,17 @@ public class SlingScriptEngineManager extends ScriptEngineManager implements Bun
         }
     }
 
+    private ScriptEngineManager getInternalScriptEngineManager() {
+        ClassLoader loader = Thread.currentThread().getContextClassLoader();
+        try {
+            Thread.currentThread().setContextClassLoader(null);
+            return new ScriptEngineManager(ClassLoader.getSystemClassLoader());
+        }
+        finally {
+            Thread.currentThread().setContextClassLoader(loader);
+        }
+    }
+
     private void registerAssociations(ScriptEngineFactory factory) {
         for (String extension : factory.getExtensions()) {
             internalManager.registerEngineExtension(extension, factory);
diff --git a/src/test/java/org/apache/sling/scripting/core/impl/jsr223/SlingScriptEngineManagerTest.java b/src/test/java/org/apache/sling/scripting/core/impl/jsr223/SlingScriptEngineManagerTest.java
index 33a46f4..b61757f 100644
--- a/src/test/java/org/apache/sling/scripting/core/impl/jsr223/SlingScriptEngineManagerTest.java
+++ b/src/test/java/org/apache/sling/scripting/core/impl/jsr223/SlingScriptEngineManagerTest.java
@@ -53,6 +53,7 @@ import org.osgi.service.event.EventHandler;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -91,9 +92,9 @@ public class SlingScriptEngineManagerTest {
         );
 
         ScriptEngineFactory f1 = mockScriptEngineFactory("f1", "1.0", Collections.singletonList("f1"), "f1", "1.0", Collections
-                .singletonList("text"));
+                .singletonList("f1/text"));
         ScriptEngineFactory f2 = mockScriptEngineFactory("f2", "1.0", Collections.singletonList("f2"), "f2", "1.0", Collections
-                .singletonList("text"));
+                .singletonList("f2/text"));
 
         ServiceRegistration<ScriptEngineFactory> f1SR = context.bundleContext().registerService(ScriptEngineFactory.class, f1, new
                 Hashtable<String, Object>() {{
@@ -122,7 +123,13 @@ public class SlingScriptEngineManagerTest {
 
         assertEquals(f2, scriptEngineManager.getEngineByName("f2").getFactory());
         assertEquals(f2, scriptEngineManager.getEngineByExtension("f2").getFactory());
-        assertEquals(f2, scriptEngineManager.getEngineByMimeType("text").getFactory());
+        assertEquals(f2, scriptEngineManager.getEngineByMimeType("f2/text").getFactory());
+        assertNull("Did not expect references to the already unregistered f1 ScriptEngineFactory", scriptEngineManager.getEngineByName
+                ("f1"));
+        assertNull("Did not expect references to the already unregistered f1 ScriptEngineFactory", scriptEngineManager
+                .getEngineByExtension("f1"));
+        assertNull("Did not expect references to the already unregistered f1 ScriptEngineFactory", scriptEngineManager
+                .getEngineByMimeType("f1/text"));
 
         latch.await(2, TimeUnit.SECONDS);
         assertEquals("Expected a different number of processed " + SlingScriptEngineManager.EVENT_TOPIC_SCRIPT_MANAGER_UPDATED + " events.",
@@ -136,13 +143,13 @@ public class SlingScriptEngineManagerTest {
         BundleWiring wiring = mock(BundleWiring.class);
         ClassLoader loader = new SecureClassLoader(){
             @Override
-            protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
+            protected Class<?> loadClass(String name, boolean resolve) {
                 return name.equals(SCRIPT_ENGINE_FACTORY.getName()) ? SCRIPT_ENGINE_FACTORY : null;
             }
 
             @Override
-            public Enumeration<URL> getResources(String name) throws IOException {
-                Vector v = new Vector();
+            public Enumeration<URL> getResources(String name) {
+                Vector<URL> v = new Vector<>();
                 v.add(url);
                 return v.elements();
             }
@@ -167,6 +174,12 @@ public class SlingScriptEngineManagerTest {
         factories = slingScriptEngineManager.getEngineFactories();
         assertEquals("Expected 1 ScriptEngineFactory.", 1, factories.size());
         assertEquals("Oracle Nashorn", factories.get(0).getEngineName());
+        assertNull("Did not expect references to the already unregistered DummyScriptEngineFactory", slingScriptEngineManager
+                .getEngineByExtension("dummy"));
+        assertNull("Did not expect references to the already unregistered DummyScriptEngineFactory",
+                slingScriptEngineManager.getEngineByMimeType("application/x-dummy"));
+        assertNull("Did not expect references to the already unregistered DummyScriptEngineFactory",
+                slingScriptEngineManager.getEngineByName("Dummy"));
     }
 
     private ScriptEngineFactory mockScriptEngineFactory(String engineName, String engineVersion, List<String> extensions, String

-- 
To stop receiving notification emails like this one, please contact
['"commits@sling.apache.org" <co...@sling.apache.org>'].