Closed Bug 15197 Opened 25 years ago Closed 25 years ago

setTimeout() is still called after window is closed

Categories

(Core :: JavaScript Engine, defect, P1)

Product:
Core
Component:
JavaScript Engine
Type:
defect

Tracking

()

Status:
VERIFIED DUPLICATE of bug 15261

People

(Reporter: dougt, Assigned: rogerl)

Details

This javascript crashes in nsJSEnvironment.cpp, line 80 or so, because 'owner'
is invalid:

<html>
<script language=javascript>

var doclose=false;

function closewin()
{
    doclose = true;
}

function closeif()
{
    if (doclose)
    {
        window.close();
    }
    setTimeout("closeif()", 100);
}

setTimeout("closeif()", 100);

</script>

<form>
<input type="button" value="TouchMe" onClick="closewin()">
</form>
</body>
</html>
Priority: P3 → P1
Target Milestone: M11
proposed fix:

Index: nsJSEnvironment.cpp
===================================================================
RCS file: /cvsroot/mozilla/dom/src/base/nsJSEnvironment.cpp,v
retrieving revision 1.48
diff -c -4 -r1.48 nsJSEnvironment.cpp
*** nsJSEnvironment.cpp 1999/09/17 20:13:24     1.48
--- nsJSEnvironment.cpp 1999/09/30 21:36:00
***************
*** 65,73 ****
    nsIScriptContextOwner* owner;

    if (context) {
      nsresult result = context->GetOwner(&owner);
!     if (NS_SUCCEEDED(result)) {
        const char* error;
        if (message) {
          error = message;
        }
--- 65,73 ----
    nsIScriptContextOwner* owner;

    if (context) {
      nsresult result = context->GetOwner(&owner);
!     if (NS_SUCCEEDED(result) && owner) {
        const char* error;
        if (message) {
          error = message;
        }
Status: NEW → ASSIGNED
Well, I'd like to see why the timeout isn't canceled in time. Looking at the bug
right now...
The window.close() in the timeout is running synchronously (this is different
from 4.x, where the JS<->UI thread separation changed the timing of this) and
causing a JS_ClearScope to be called while there's still more script to be
executed. In this case, the setTimeout following the if block in the callback
fails (it wouldn't if it were surrounded by an else clause) because it's been
cleared. As a result, an exception is thrown. We crash in jsexn.c, trying to
convert the uncaught exception into a string. The following patch, along with
dougt's into nsJSException will fix the crash:

Index: jsexn.c
===================================================================
RCS file: /cvsroot/mozilla/js/src/jsexn.c,v
retrieving revision 3.8
diff -r3.8 jsexn.c
617a618
>     const char* bytes;
637a639,645
>     if (str != NULL) {
>         bytes = js_GetStringBytes(str);
>     }
>     else {
>         bytes = "null";
>     }
>
651c659
<                              JSMSG_UNCAUGHT_EXCEPTION, js_GetStringBytes(str))
;
---
>                              JSMSG_UNCAUGHT_EXCEPTION, bytes);
655c663
<         js_ReportErrorAgain(cx, js_GetStringBytes(str), reportp);
---
>         js_ReportErrorAgain(cx, bytes, reportp);
Reassigning to rogerl to review, update and checkin the suggested patch to
js/src.

DougT - please check in your suggested change citing me as the reviewer.

Still need to think about other ramifications of running the window.close()
synchronously. Brendan, any thoughts?
Status: NEW → RESOLVED
Closed: 25 years ago
Resolution: --- → DUPLICATE
This seems to be a duplicate of bug 15261? while the suggested fix is kosher, my
concern is tracking down the origin of the null string itself.

*** This bug has been marked as a duplicate of 15261 ***
any ETA on when this patch (or something like it) will be checked in?

this it bitting me in a number of places, and window.close() related.
So far I'm getting nowhere on reproducing the null exception string here at
home, if you want to apply vidur's patch to get you going I have no objection,
but I expect to have this resolved tonight or tomorrow morning.
I've checked in vidur's patch, with rogerl's blessing.

rogerl said he would continue investigating to see why its happening.
Status: RESOLVED → VERIFIED
doesn't happen for me anymore.  thanks.
Component: Java APIs to WebShell → Javascript Engine
You need to log in before you can comment on or make changes to this bug.