unresponsive scripts in sandboxes associated with the hidden window are never terminated

RESOLVED FIXED in mozilla17

Status

()

Core
DOM
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Gavin, Assigned: Gavin)

Tracking

unspecified
mozilla17
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

The FrameWorker component created in bug 762569 creates iframes in the hidden window, and uses them as contexts for sandboxes, which then evaluate scripts retrieved from the social providers.

It's possible for the social provider's script to have bugs that could trigger an infinite loop, or otherwise unresponsive script. Right now, when this happens the browser becomes entirely unresponsive with no recourse - the slow script dialog doesn't appear, presumably because it can't usefully parent itself to the hidden window.

We need some mechanism to ensure that these situations don't result in a totally hosed browser. One way to achieve this may be to just cancel the script automatically if the slow script dialog fails to display, or something like that.

Related note: it would be awesome if we could somehow detect that the script had been killed, so that we can disable the guilty provider and/or notify the user.
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #0)
> One way to achieve this may be to just cancel the
> script automatically if the slow script dialog fails to display, or
> something like that.

Hmm, this already seems to be what nsJSContext::DOMOperationCallback does. I'll investigate why this isn't working.
Oh, I was wrong, the case being hit is this one:

  rv = prompt->ConfirmEx(title, msg, buttonFlags, waitButton, stopButton,
                         debugButton, neverShowDlg, &neverShowDlgChk,
                         &buttonPressed);
...
  if (NS_FAILED(rv) || (buttonPressed == 0)) {
    // Allow the script to continue running

NS_FAILED(rv) is true, so we let the script continue running.

Can we just change that? What cases would we fail to prompt and want to continue running unresponsive scripts? This behavior seems to go back all the way to bug 13350, and I don't know that it's ever mattered in practice before now.
Summary: need way to terminate (and observe?) unresponsive script in sandboxes associated with the hidden window → unresponsive scripts in sandboxes associated with the hidden window are never terminated
Created attachment 640143 [details] [diff] [review]
proposed patch

This is the minimal workable solution - terminates the runaway script after the normal prompt timeout.

Ideally, we'd limit these scripts even further than that, and have some way to detect this occurring.
Attachment #640143 - Flags: review?(jst)
Component: XPConnect → DOM
Comment on attachment 640143 [details] [diff] [review]
proposed patch

I'm generally fine with this change, it means when unexpected things fails, we terminate scripts, as opposed to what we do now which is to let them run. I don't think we ever had a case where we needed this to work one way or another, it just seemed to be the safer bet, but in this case it's not. Alternatively we could of course write code that would explicitly check if we're running in a sandbox and make a decision based on that, but I don't think that's worth the effort right now.
Attachment #640143 - Flags: review?(jst) → review+
Assignee: nobody → gavin.sharp
https://tbpl.mozilla.org/?tree=Try&rev=3124715357b3
https://hg.mozilla.org/integration/mozilla-inbound/rev/75b6ec6db468
Target Milestone: --- → mozilla17
Flags: in-testsuite-

Comment 7

5 years ago
https://hg.mozilla.org/mozilla-central/rev/75b6ec6db468
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.