Bug 166875 – Crash when WebCore's GC heap grows way too large.

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Andreas Kling]

<rdar://problem/27896585>

Let's put a hard limit on the GC and crash instead of descending into swap hell.

[Andreas Kling]

Created attachment 298442 [details]
Proposed patch

[Mark Lam]

Comment on attachment 298442 [details]
Proposed patch

r=me

[WebKit Commit Bot]

Comment on attachment 298442 [details]
Proposed patch

Clearing flags on attachment: 298442

Committed r210540: <http://trac.webkit.org/changeset/210540>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Filip Pizlo]

Comment on attachment 298442 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=298442&action=review

> Source/JavaScriptCore/ChangeLog:10
> +        Hard cap the JavaScript heap at 4GB of live objects (determined post-GC.)
> +        If we go past this limit, crash with a recognizable signature.

Why?

This is a regression.  If I have 20GB of RAM, why I can't I write a JavaScript program that uses all of it?  Previously, I could.  Now, I can't.

[Filip Pizlo]

If the point is to give a quota to WebProcesses, then the max heap size should be a parameter that WebCore passes to JSC.

When JSC is used as a framework, embedded in who-knows-what, I don't think we should have any such limit.

[Mark Lam]

(In reply to comment #6)
> If the point is to give a quota to WebProcesses, then the max heap size
> should be a parameter that WebCore passes to JSC.
> 
> When JSC is used as a framework, embedded in who-knows-what, I don't think
> we should have any such limit.

That's a very good point.

[Andreas Kling]

Yeah, fair point Phil. I will roll this out and give it a do-over.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 166896

[Andreas Kling]

Created attachment 298597 [details]
Proposed patch

Only the common WebCore VM this time.

[Mark Lam]

Comment on attachment 298597 [details]
Proposed patch

r=me

[Andreas Kling]

Created attachment 298605 [details]
Patch for landing (Windows build fixed)

[Geoffrey Garen]

Why not apply to the limit to the WebContent process instead?

[Andreas Kling]

(In reply to comment #13)
> Why not apply to the limit to the WebContent process instead?

The long-term plan is to integrate with OS facilities to do that (and more.)

This patch is just a stopgap mitigation for a specific pathology.

[WebKit Commit Bot]

Comment on attachment 298605 [details]
Patch for landing (Windows build fixed)

Clearing flags on attachment: 298605

Committed r210609: <http://trac.webkit.org/changeset/210609>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.