Bug 158290

Summary: Runaway WebContent process CPU & memory @ foxnews.com
Product: WebKit Reporter: Michael Saboff <msaboff>
Component: JavaScriptCoreAssignee: Michael Saboff <msaboff>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, keith_miller, mark.lam, ossy, saam
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch mark.lam: review+

Description Michael Saboff 2016-06-01 17:58:22 PDT
Some foxnews.com pages with embedded videos have this code:

start : function() {
    if (this.started === true) {
        return;
    }
    try {
        this.plugin.Media.open(this.getMediaName(), this.media.getDuration(), this.getPlayerName()); // <== Exception here
        this.started = true;
    } catch (error) {
        this.error(error);
    }
    this.play(); // recursively call play()
}

play : function() {
    if (this.started !== true) {
        this.start(); // <== call here
        return;
    }
    try {
        this.plugin.Media.play(this.getMediaName(), this.getPlaybackCore().getCurrentTime() || 0);
    } catch (error) {
        this.error(error);
    }
}

This can end up being a mutual recursive call chain that will eventually exceed the stack space.  This happens when this.plugin.Media.open() in start() throws an exception.  That exception is caught in the catch block, but start() then calls play().  At the top of play(), it will recursively call start() and we end up in a recursive loop.  While the stack grows, the amount of memory used goes up dramatically.  The memory use is due to the error objects that are created for each throw.  The catch will be handled in baseline JIT'ed code as the higher tiers will ORS exit at the start of their catch blocks.  The baseline JIT'ed code will allocate a local for the thrown value and the conservative scan of the stack during GC will include the thrown value.  This means that the error objects for all start() frames further up the stack will not be collected.
Comment 1 Michael Saboff 2016-06-01 17:58:57 PDT
<rdar://problem/24756019>
Comment 2 Michael Saboff 2016-06-01 18:06:10 PDT
Created attachment 280288 [details]
Patch
Comment 3 Mark Lam 2016-06-01 19:01:40 PDT
Comment on attachment 280288 [details]
Patch

r=me
Comment 4 Michael Saboff 2016-06-01 21:05:54 PDT
Committed r201589: <http://trac.webkit.org/changeset/201589>
Comment 5 Csaba Osztrogonác 2016-06-03 07:33:28 PDT
(In reply to comment #4)
> Committed r201589: <http://trac.webkit.org/changeset/201589>

stress/recursive-try-catch.js.ftl-no-cjit-validate-sampling-profiler times out on Apple Mac 32 bit debug bot: https://build.webkit.org/builders/Apple%20El%20Capitan%2032-bit%20JSC%20%28BuildAndTest%29/builds/2545