#1779 HtmlUnit hangs when using more windows and javascript

[dmatej]

Cause: Some windows has parent scopes set to each other. Then getTopLevelScope ends up in never ending loop.
I'm not sure if my fix is correct but it works with that, our selenium tests passed.

I think I got it and perhaps got it fixed (but not nice). One part is in HtmlUnit and another in HtmlUnit's Rhino fork on GitHub. I'm not sure if it is this the same thing, but I can post you this stacktrace (obtained with the help of some temporary code in Window.setParentScope to get some idea where is the cause):

19:30:48,363 ERROR [JS executor for com.gargoylesoftware.htmlunit.WebClient@28c2c4a8] com.gargoylesoftware.htmlunit.javascript.background.JavaScriptJobManagerImpl:429 : Job run failed with unexpected RuntimeException: Detected cycle in parent scopes for window: com.gargoylesoftware.htmlunit.javascript.host.Window@7d9a83d
======= EXCEPTION START ========
Exception class=[java.lang.IllegalStateException]
com.gargoylesoftware.htmlunit.ScriptException: Detected cycle in parent scopes for window: com.gargoylesoftware.htmlunit.javascript.host.Window@7d9a83d
        at com.gargoylesoftware.htmlunit.javascript.JavaScriptEngine$HtmlUnitContextAction.run(JavaScriptEngine.java:894)
        at net.sourceforge.htmlunit.corejs.javascript.Context.call(Context.java:613)
        at net.sourceforge.htmlunit.corejs.javascript.ContextFactory.call(ContextFactory.java:539)
        at com.gargoylesoftware.htmlunit.javascript.JavaScriptEngine.callFunction(JavaScriptEngine.java:825)
        at com.gargoylesoftware.htmlunit.javascript.host.xml.XMLHttpRequest.setState(XMLHttpRequest.java:205)
        at com.gargoylesoftware.htmlunit.javascript.host.xml.XMLHttpRequest.doSend(XMLHttpRequest.java:786)
        at com.gargoylesoftware.htmlunit.javascript.host.xml.XMLHttpRequest.access$000(XMLHttpRequest.java:98)
        at com.gargoylesoftware.htmlunit.javascript.host.xml.XMLHttpRequest$1.run(XMLHttpRequest.java:640)
        at net.sourceforge.htmlunit.corejs.javascript.Context.call(Context.java:613)
        at net.sourceforge.htmlunit.corejs.javascript.ContextFactory.call(ContextFactory.java:539)
        at com.gargoylesoftware.htmlunit.javascript.background.JavascriptXMLHttpRequestJob.run(JavascriptXMLHttpRequestJob.java:36)
        at com.gargoylesoftware.htmlunit.javascript.background.JavaScriptJobManagerImpl.runSingleJob(JavaScriptJobManagerImpl.java:426)
        at com.gargoylesoftware.htmlunit.javascript.background.DefaultJavaScriptExecutor.run(DefaultJavaScriptExecutor.java:156)
        at java.lang.Thread.run(Thread.java:745)
Caused by: java.lang.IllegalStateException: Detected cycle in parent scopes for window: com.gargoylesoftware.htmlunit.javascript.host.Window@7d9a83d
        at com.gargoylesoftware.htmlunit.javascript.host.Window.setParentScope(Window.java:226)
        at net.sourceforge.htmlunit.corejs.javascript.BaseFunction.construct(BaseFunction.java:382)
        at net.sourceforge.htmlunit.corejs.javascript.NativeObject.execIdCall(NativeObject.java:118)
        at net.sourceforge.htmlunit.corejs.javascript.IdFunctionObject.call(IdFunctionObject.java:101)
        at net.sourceforge.htmlunit.corejs.javascript.Interpreter.interpretLoop(Interpreter.java:1479)
        at net.sourceforge.htmlunit.corejs.javascript.Interpreter.interpret(Interpreter.java:815)
        at net.sourceforge.htmlunit.corejs.javascript.InterpretedFunction.call(InterpretedFunction.java:111)
        at net.sourceforge.htmlunit.corejs.javascript.ContextFactory.doTopCall(ContextFactory.java:429)
        at com.gargoylesoftware.htmlunit.javascript.HtmlUnitContextFactory.doTopCall(HtmlUnitContextFactory.java:252)
        at net.sourceforge.htmlunit.corejs.javascript.ScriptRuntime.doTopCall(ScriptRuntime.java:3345)
        at com.gargoylesoftware.htmlunit.javascript.JavaScriptEngine$4.doRun(JavaScriptEngine.java:818)
        at com.gargoylesoftware.htmlunit.javascript.JavaScriptEngine$HtmlUnitContextAction.run(JavaScriptEngine.java:879)

The problem is that we use two windows, one opens the second, the second has an active content changed by javascript. That causes the setParentScope call which causes a cycle between windows.
Subsequent call of getTopLevelScope method hangs, because windows point to each other and neither one has null set as parent scope.

So I did this in HtmlUnitScriptable:

• prevent cycles but don't crash the whole system (still don't know how to fix it better).

    public void setParentScope(final Scriptable m) {
        final Scriptable oldScope = getParentScope();
        super.setParentScope(m);

        final Scriptable topLevelScope = getTopLevelScope(this);
        if (topLevelScope.getParentScope() != null) {
            LOG.warn("Detected cycle in parent scopes for window: " + this + ", ignoring the call of this setter.");
            super.setParentScope(oldScope);
            return;
        }
    }

And I did this in ScriptableObject:

• detect cycles and return first item the was seen twice.
• still caused failing assert in TopLevel.getBuiltinPrototype without other changes.

    public static Scriptable getTopLevelScope(final Scriptable obj)
    {
        // FIXME: some classes throw NPE in toString methods!
        // FIXME: some classes throw StackOverflow in toString methods!
        System.out.println("getTopLevelScope: obj.class=" + obj.getClass());
        final HashSet<Scriptable> cache = new HashSet<Scriptable>();
        cache.add(obj);
        Scriptable temp = obj;
        for (;;) {
            final Scriptable parent = temp.getParentScope();
            System.out.println("parent: " + parent);
            // prevent indefinite loop.
            // FIXME: HtmlUnit's Window handles are sometimes each other's descendants.
            if (cache.contains(parent)) {
                return parent;
//              throw new IllegalStateException("Scriptable is it's own descendant!: " + obj);
            }
            if (parent == null) {
                return temp;
            }
            temp = parent;
            cache.add(parent);
        }
    }

And this in Window:

• window of this window is this window, not it's parent.

    /**
     * @return this instance.
     */
    @Override
    public Window getWindow() throws RuntimeException {
      return this;
    }

[Ahmed Ashour]

Hi David,

Thanks for your investigations, we can commit it bit by bit.

Please share your case, so others can reproduce the error.

[Ahmed Ashour]

Please provide minimal test case.

[dmatej]

I don't know how to create such a minimal test case because it took me two days to find the cause with the help of our tests. Cause is visible in the code and described in this ticket, and I have also provided the code that fixes the problem.
I have tried newest versions of htmlunit and htmlunit-core-js but they will hang too.

[dmatej]

Pull request in Rhino fork: https://github.com/HtmlUnit/htmlunit-rhino-fork/pull/5
The scenario is using popup windows and calling an action in parent window from there via javascript and then closing the popup window.

[Ahmed Ashour]

Hi,

Window consturctor has been changed to throw exception when triggered by JavaScript (as real browsers).

I wonder which JavaScript calls this:

~~~
at com.gargoylesoftware.htmlunit.javascript.host.Window.setParentScope(Window.java:226)
at net.sourceforge.htmlunit.corejs.javascript.BaseFunction.construct(BaseFunction.java:382)
at net.sourceforge.htmlunit.corejs.javascript.NativeObject.execIdCall(NativeObject.java:118)
~~~

which hints that Window.constructor was called.

• Can you post your complete case (even if not minimal)?
• Or test with trunk version, and debug the area of BaseFunction to construct Window, this shouldn't occur
• Or provide the JavaScript code sent to JavaScriptEngine.execute(InteractivePage, String, String, int)

Making the suggested changes are dangerous without a test case, and we are also shifting from Rhino to Nashorn, so these changes would be discarded later anyway.