SLING-12516 - Migrate SlingContext to Rhino Context(ContextFactory) API

src/main/java/org/apache/sling/scripting/javascript/helper/SlingContext.java

@@ -19,6 +19,7 @@
 package org.apache.sling.scripting.javascript.helper;
 
 import org.mozilla.javascript.Context;
+import org.mozilla.javascript.ContextFactory;
 import org.mozilla.javascript.ImporterTopLevel;
 import org.mozilla.javascript.ScriptableObject;
 
@@ -29,6 +30,10 @@
  */
 public class SlingContext extends Context {
 
+    protected SlingContext(ContextFactory factory) {
+        super(factory);
+    }
+
     @Override
     public ScriptableObject initStandardObjects(ScriptableObject scope, boolean sealed) {
         ScriptableObject rootScope = super.initStandardObjects(scope, sealed);

src/main/java/org/apache/sling/scripting/javascript/helper/SlingContextFactory.java

@@ -18,7 +18,7 @@
  */
 package org.apache.sling.scripting.javascript.helper;
 
-import java.lang.reflect.Field;
+import java.util.concurrent.atomic.AtomicReference;
 
 import org.mozilla.javascript.Context;
 import org.mozilla.javascript.ContextFactory;
@@ -45,47 +45,41 @@ public class SlingContextFactory extends ContextFactory {
 
     private int languageVersion;
 
-    // conditionally setup the global ContextFactory to be ours. If
-    // a global context factory has already been set, we have lost
-    // and cannot set this one.
-    public static void setup(ScopeProvider sp, int languageVersion) {
-        // TODO what do we do in the other case? debugger won't work
-        if (!hasExplicitGlobal()) {
-            initGlobal(new SlingContextFactory(
-                    sp, Context.isValidLanguageVersion(languageVersion) ? languageVersion : Context.VERSION_DEFAULT));
-        }
-    }
-
-    public static void teardown() {
-        ContextFactory factory = getGlobal();
-        if (factory instanceof SlingContextFactory) {
-            ((SlingContextFactory) factory).dispose();
+    private static final AtomicReference<SlingContextFactory> singleSlingContextFactoryRef = new AtomicReference<>();
+
+    /**
+     * @param sp the scope provider
+     * @param languageVersion the language version.
+     * @return the SlingContextFactory instance that has been successfully registered as the custom global context factory of the Rhino Runtime.
+     */
+    public static SlingContextFactory getInstance(ScopeProvider sp, int languageVersion) throws RuntimeException {
+        if (singleSlingContextFactoryRef.get() == null) {
+            synchronized (SlingContextFactory.class) {
+                if (singleSlingContextFactoryRef.get() == null) {
+                    try {
+                        SlingContextFactory factory = new SlingContextFactory(sp, languageVersion);
+                        ContextFactory.initGlobal(factory);
+                        singleSlingContextFactoryRef.set(factory);
+                    } catch (IllegalArgumentException e) {
+                        throw new RuntimeException("Fail to set Sling Context Factory as custom context factory", e);
+                    } catch (IllegalStateException e) {
+                        throw new RuntimeException("Custom context factory already initiated.", e);
+                    }
+                }
+            }
         }
+        return singleSlingContextFactoryRef.get();
     }
 
-    // private as instances of this class are only used by setup()
     private SlingContextFactory(ScopeProvider sp, int languageVersion) {
         scopeProvider = sp;
-        this.languageVersion = languageVersion;
-    }
-
-    private void dispose() {
-        // ensure the debugger is closed
-        exitDebugger();
-
-        // reset the context factory class for future use
-        ContextFactory newGlobal = new ContextFactory();
-        setField(newGlobal, "hasCustomGlobal", Boolean.FALSE);
-        setField(newGlobal, "global", newGlobal);
-        setField(newGlobal, "sealed", Boolean.FALSE);
-        setField(newGlobal, "listeners", null);
-        setField(newGlobal, "disabledListening", Boolean.FALSE);
-        setField(newGlobal, "applicationClassLoader", null);
+        this.languageVersion =
+                Context.isValidLanguageVersion(languageVersion) ? languageVersion : Context.VERSION_DEFAULT;
     }
 
     @Override
     protected Context makeContext() {
-        Context context = new SlingContext();
+        Context context = new SlingContext(this);
         context.setLanguageVersion(languageVersion);
         return context;
     }
@@ -139,20 +133,4 @@ public void setDebugging(boolean enable) {
     public boolean isDebugging() {
         return debuggerActive;
     }
-
-    private void setField(Object instance, String fieldName, Object value) {
-        try {
-            Field field = instance.getClass().getDeclaredField(fieldName);
-            if (!field.isAccessible()) {
-                field.setAccessible(true);
-            }
-            field.set(instance, value);
-        } catch (IllegalArgumentException iae) {
-            // don't care, but it is strange anyhow
-        } catch (IllegalAccessException iae) {
-            // don't care, but it is strange anyhow
-        } catch (NoSuchFieldException nsfe) {
-            // don't care, but it is strange anyhow
-        }
-    }
 }

src/main/java/org/apache/sling/scripting/javascript/internal/RhinoJavaScriptEngineFactory.java

@@ -51,7 +51,6 @@
 import org.apache.sling.scripting.javascript.wrapper.ScriptableVersion;
 import org.apache.sling.scripting.javascript.wrapper.ScriptableVersionHistory;
 import org.mozilla.javascript.Context;
-import org.mozilla.javascript.ContextFactory;
 import org.mozilla.javascript.ImporterTopLevel;
 import org.mozilla.javascript.NativeJavaClass;
 import org.mozilla.javascript.NativeJavaPackage;
@@ -125,7 +124,7 @@ public class RhinoJavaScriptEngineFactory extends AbstractScriptEngineFactory im
 
     private int optimizationLevel;
 
-    private static final int RHINO_LANGUAGE_VERSION = Context.VERSION_ES6;
+    public static final int RHINO_LANGUAGE_VERSION = Context.VERSION_ES6;
     private static final String LANGUAGE_VERSION = "partial ECMAScript 2015 support";
     private static final String LANGUAGE_NAME = "ECMAScript";
 
@@ -135,6 +134,8 @@ public class RhinoJavaScriptEngineFactory extends AbstractScriptEngineFactory im
 
     private final Set<RhinoHostObjectProvider> hostObjectProvider = new HashSet<RhinoHostObjectProvider>();
 
+    private SlingContextFactory contextFactory;
+
     @Reference
     private DynamicClassLoaderManager dynamicClassLoaderManager = null;
 
@@ -252,10 +253,7 @@ private void dropRootScope() {
         // ensure the debugger is closed if the root scope will
         // be replaced to ensure no references to the old scope
         // and context remain
-        ContextFactory contextFactory = ContextFactory.getGlobal();
-        if (contextFactory instanceof SlingContextFactory) {
-            ((SlingContextFactory) contextFactory).exitDebugger();
-        }
+        contextFactory.exitDebugger();
 
         // drop the scope
         rootScope = null;
@@ -308,26 +306,35 @@ protected void activate(
             wrapFactory = new SlingWrapFactory();
 
             // initialize the Rhino Context Factory
-            SlingContextFactory.setup(this, RHINO_LANGUAGE_VERSION);
+            contextFactory = SlingContextFactory.getInstance(this, RHINO_LANGUAGE_VERSION);
+            contextFactory.setDebugging(debugging);
 
             setEngineName(getEngineName() + " (Rhino " + (rhinoVersion != null ? rhinoVersion : "unknown") + ")");
 
             setExtensions(PropertiesUtil.toStringArray(props.get("extensions")));
             setMimeTypes(PropertiesUtil.toStringArray(props.get("mimeTypes")));
             setNames(PropertiesUtil.toStringArray(props.get("names")));
 
-            final ContextFactory contextFactory = ContextFactory.getGlobal();
-            if (contextFactory instanceof SlingContextFactory) {
-                ((SlingContextFactory) contextFactory).setDebugging(debugging);
-            }
             // set the dynamic class loader as the application class loader
             final DynamicClassLoaderManager dclm = this.dynamicClassLoaderManager;
             if (dclm != null) {
-                contextFactory.initApplicationClassLoader(dynamicClassLoaderManager.getDynamicClassLoader());
+                if (contextFactory.getApplicationClassLoader() == null) {
+                    contextFactory.initApplicationClassLoader(dynamicClassLoaderManager.getDynamicClassLoader());
+                } else {
+                    log.warn("Ignoring multiple dynamic class loader registration attempts.");
+                }
             }
 
             log.info("Activated with optimization level {}", optimizationLevel);
             active = true;
+        } catch (RuntimeException e) {
+            /**
+             * Fail the bundle initialization if SlingContextFactory was not registered as the custom context
+             * provide to prevent incorrect scripting processing results.
+             */
+            log.error(
+                    "Cannot register SlingContexrtFactory as custom global context provider.  This could happen if there is another Rhino JavaScript service conflict.");
+            throw e;
         } finally {
             writeLock.unlock();
         }
@@ -341,10 +348,8 @@ protected void deactivate(ComponentContext context) {
             // remove the root scope
             dropRootScope();
 
-            // remove our context factory
-            SlingContextFactory.teardown();
-
             // remove references
+            contextFactory = null;
             wrapFactory = null;
             hostObjectProvider.clear();
 

src/test/java/org/apache/sling/scripting/javascript/internal/RhinoJavaScriptEngineFactoryTest.java

@@ -24,10 +24,12 @@
 
 import org.apache.sling.commons.classloader.DynamicClassLoaderManager;
 import org.apache.sling.scripting.api.ScriptCache;
+import org.apache.sling.scripting.javascript.helper.SlingContextFactory;
 import org.apache.sling.testing.mock.osgi.junit5.OsgiContext;
 import org.apache.sling.testing.mock.osgi.junit5.OsgiContextExtension;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
+import org.mozilla.javascript.ContextFactory;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -58,4 +60,24 @@ void testRegistrationProperties() {
                 instance.getEngineName() != null && instance.getEngineName().contains("Rhino 1.7.7.1_1"),
                 "Unexpected engine name");
     }
+
+    @Test
+    void testGlobalSlingContextFactory() {
+        DynamicClassLoaderManager dynamicClassLoaderManager = mock(DynamicClassLoaderManager.class);
+        when(dynamicClassLoaderManager.getDynamicClassLoader())
+                .thenReturn(RhinoJavaScriptEngineFactoryTest.class.getClassLoader());
+        context.registerService(DynamicClassLoaderManager.class, dynamicClassLoaderManager);
+        context.registerService(ScriptCache.class, mock(ScriptCache.class));
+        context.registerInjectActivateService(new RhinoJavaScriptEngineFactory());
+        RhinoJavaScriptEngineFactory instance =
+                (RhinoJavaScriptEngineFactory) context.getService(ScriptEngineFactory.class);
+        assertTrue(
+                (ContextFactory.getGlobal() instanceof SlingContextFactory),
+                "SlingContextFactory type should be the global context factory but is not");
+        assertTrue(
+                ContextFactory.getGlobal()
+                        == (ContextFactory) (SlingContextFactory.getInstance(
+                                instance, RhinoJavaScriptEngineFactory.RHINO_LANGUAGE_VERSION)),
+                "SlingContextFactory instance should always equal to the registered =global context factory but is not");
+    }
 }