Merge branch 'master' into reorganize

rhino-runtime/src/main/java/org/mozilla/javascript/ScriptableObject.java

@@ -19,10 +19,12 @@
 import java.lang.reflect.Member;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.function.Consumer;
@@ -1990,21 +1992,40 @@ public void preventExtensions() {
      * @since 1.4R3
      */
     public void sealObject() {
-        if (!isSealed) {
-            final long stamp = slotMap.readLock();
+        // We cannot initialize LazyLoadedCtor while holding the read lock, since they will mutate
+        // the current object (case in point: NativeJavaTopPackage), which in turn would cause the
+        // code
+        // to try to acquire a write lock while holding the read lock... and deadlock.
+        // We do this in a loop because an initialization could add _other_ stuff to initialize.
+
+        List<Slot> toInitialize = new ArrayList<>();
+        while (!isSealed) {
+            for (Slot slot : toInitialize) {
+                // Need to check the type again, because initializing one slot _could_ have
+                // initialized another one
+                Object value = slot.value;
+                if (value instanceof LazilyLoadedCtor) {
+                    LazilyLoadedCtor initializer = (LazilyLoadedCtor) value;
+                    try {
+                        initializer.init();
+                    } finally {
+                        slot.value = initializer.getValue();
+                    }
+                }
+            }
+            toInitialize.clear();
+
+            long stamp = slotMap.readLock();
             try {
                 for (Slot slot : slotMap) {
                     Object value = slot.value;
                     if (value instanceof LazilyLoadedCtor) {
-                        LazilyLoadedCtor initializer = (LazilyLoadedCtor) value;
-                        try {
-                            initializer.init();
-                        } finally {
-                            slot.value = initializer.getValue();
-                        }
+                        toInitialize.add(slot);
                     }
                 }
-                isSealed = true;
+                if (toInitialize.isEmpty()) {
+                    isSealed = true;
+                }
             } finally {
                 slotMap.unlockRead(stamp);
             }

rhino-runtime/src/test/java/org/mozilla/javascript/ThreadSafeScriptableObjectTest.java

@@ -0,0 +1,30 @@
+package org.mozilla.javascript;
+
+import static org.junit.Assert.assertNotNull;
+
+import org.junit.Test;
+
+public class ThreadSafeScriptableObjectTest {
+    @Test
+    public void canSealGlobalObjectWithoutDeadlock() {
+        ContextFactory.getGlobalSetter()
+                .setContextFactoryGlobal(
+                        new ContextFactory() {
+                            @Override
+                            protected boolean hasFeature(Context cx, int featureIndex) {
+                                if (featureIndex == Context.FEATURE_THREAD_SAFE_OBJECTS) {
+                                    return true;
+                                }
+                                return super.hasFeature(cx, featureIndex);
+                            }
+                        });
+
+        try (Context cx = Context.enter()) {
+            ScriptableObject global = cx.initStandardObjects();
+            global.sealObject();
+
+            // Registered by NativeJavaTopPackage
+            assertNotNull(global.get("Packages", global));
+        }
+    }
+}