Bug 29797 – [V8] Runaway JS recursion crashes Chromium workers on OS X

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Dominic Cooney]

Created attachment 40218 [details]
Sets a stack limit. Do not land before proposed fix lands in V8--see description.

Repro: Open LayoutTests/fast/workers/use-machine-stack.html.

Expected behavior: it prints PASS (RangeError: Maximum call stack size exceeded.)

Actual behavior:
Safari 4.0.3 (6531.9): OK.
Google Chrome 4.0.212.1 (Official Build 26852) WebKit 532.1 V8 1.3.11.1: Worker process crashes.

This is related to <http://crbug.com/21653>.

I've attached a patch. The patch depends on a proposed fix to V8 bug 442, see <http://codereview.chromium.org/242014>.

[Dimitri Glazkov (Google)]

We typically don't modify tests to fix them. Can you explain why we have to change the test? Is it failing for JSC?

[Dominic Cooney]

It isn't necessary to change the test. I added a stack overflow in a setTimeout callback to exercise stack overflow on another code path. I could revert the test change... either way.

The current test and the updated test pass with WebKit.

[Dimitri Glazkov (Google)]

Comment on attachment 40218 [details]
Sets a stack limit. Do not land before proposed fix lands in V8--see description.

> +        Reviewed by NOBODY (OOPS!).
> +
> +        Sets a stack limit for V8 workers.
Need a better description of this test change and a bug URL.


>  
> +function h() {
> +    try {
> +        g();
> +    } catch (ex) {
> +        postMessage(ex);
> +    }
> +}
> +
>  try {
>      f();
>  } catch (ex) {
>      try {
>          g();
>      } catch (ex) {
> -        postMessage(ex);
> +        setTimeout(h, 0);
>      }

perhaps this could be a new test?

> +        Sets a stack limit for V8 workers.

Bug url is needed here. Also, make sure to prefix V8-related bug titles with [V8].

[Dominic Cooney]

Runaway recursion in JavaScript workers crashes the Chromium worker process on OS X. This is because V8's default stack limit is 512K on ia32 or 1M on x64, but the worker process runs workers on a thread with the OS X default stack size--512K. Because there are already some C++ frames on the stack when V8 establishes its 512K default stack limit, and V8 doesn't precisely enforce the stack limit, runaway recursion in V8 workers overflows the OS stack and segfaults, killing the worker process. This is described in Chromium bug 21653 <http://crbug.com/21653>.

[Dominic Cooney]

Created attachment 40999 [details]
Proposed fix. Uses the fixed V8 resource constraints API to set a stack limit for Chromium JavaScript workers.

[Adam Barth]

Comment on attachment 40999 [details]
Proposed fix. Uses the fixed V8 resource constraints API to set a stack limit for Chromium JavaScript workers.

Thanks!  If you'd like the commit-queue to land this patch, you can nominate it by setting commit-queue=?

[WebKit Commit Bot]

Comment on attachment 40999 [details]
Proposed fix. Uses the fixed V8 resource constraints API to set a stack limit for Chromium JavaScript workers.

Clearing flags on attachment: 40999

Committed r49427: <http://trac.webkit.org/changeset/49427>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.