Bug 171630 – [JSC] Test stringimpl-to-jsstring-on-large-strings-2 uses a lot of RAM

• Status: NEW
• Component: JavaScriptCore

[Carlos Alberto Lopez Perez]

The JSC test stringimpl-to-jsstring-on-large-strings-2.js needs large amounts of RAM to run (between 3GB and 5GB on the GTK+ port)

I detected this because I just set a 16GB of RAM limit for the GTK+ release test bot (in order to detect things like this one), and this test crashed after doing that:

From https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20%28Tests%29/builds/902/steps/jscore-test/logs/stdio:
jsc-layout-tests.yaml/js/script-tests/stringimpl-to-jsstring-on-large-strings-2.js.layout-no-cjit: Killed

On the bot this was reported:
[ 1230.024807] Memory cgroup out of memory: Kill process 10486:#40003 (jsc) score 150 or sacrifice child
[ 1230.025888] Killed process 10486:#40003 (jsc) total-vm:5009940kB, anon-rss:2517060kB, file-rss:2568kB


Trying to guess if this was a WebKit bug or not I created a simple test page that just loads this test on the browser:

https://people.igalia.com/clopez/wkbug/largestrings/stringimpl-to-jsstring-on-large-strings-2.html


And the results I got are:

 - WebKit uses more than 3GB of RAM for loading that page. And the memory pressure handler doesn't do any magic here. I ran the minibrowser inside a linux cgroup memory limited to 3GB and the kernel killed the WebProcess because OOM

[479328.854217] [ pid ]   uid  tgid total_vm      rss nr_ptes nr_pmds swapents oom_score_adj name
[479328.854725] [10576]  1000 10576   547093    14894     192       5     2180             0 MiniBrowser
[479328.854733] [10596]  1000 10596   512439    12800     183       5     4157             0 WebKitNetworkPr
[479328.854737] [10599]  1000 10599  1374977   487480    1721       9   308803             0 WebKitWebProces
[479328.854749] Memory cgroup out of memory: Kill process 10599 (WebKitWebProces) score 1014 or sacrifice child

 - Chrome uses just 100MB of RAM (with a peak of 130MB of RAM) to load this very same page.

So, I think something is wrong here 

Looking at the bug that added this test, this was already reported by Ossy in its day: https://bugs.webkit.org/show_bug.cgi?id=158793#c6

[Carlos Alberto Lopez Perez]

I checked on Mac with Safari and when the page loads the memory usage peaks at ~3GB also, but it lowers after some seconds to ~230MB.

So I guess that WebKitGTK+ is not cleaning the memory properly here.


In any case, I think peaking at 3GB of RAM can be also an issue. This may cause the user's computer to start swapping if there is not enough RAM.

[Carlos Alberto Lopez Perez]

Ok.. further investigation it seems the chrome results are not valid.

The try{} on the test was hidding that chrome was simply aborting with such large string.

I have crafted a better test that will print either the time it takes to run or the exception:

https://people.igalia.com/clopez/wkbug/largestrings/test-button.html

And I get:

 - Chrome: Invalid string length
 - Firefox: repeat count must be less than infinity and not overflow maximum string size
 - WebKitGTK+: Test finished: total time to run: 1459.0900000000038 milliseconds


So, I think we should limit the string length like Chrome does....

What is the purpose of allowing such large string other than allow some malicious page to crash the user's computer?

[Carlos Alberto Lopez Perez]

(In reply to Carlos Alberto Lopez Perez from comment #2)
> Ok.. further investigation it seems the chrome results are not valid.
> 
> The try{} on the test was hidding that chrome was simply aborting with such
> large string.
> 
> I have crafted a better test that will print either the time it takes to run
> or the exception:
> 
> https://people.igalia.com/clopez/wkbug/largestrings/test-button.html
> 
> And I get:
> 
>  - Chrome: Invalid string length
>  - Firefox: repeat count must be less than infinity and not overflow maximum
> string size
>  - WebKitGTK+: Test finished: total time to run: 1459.0900000000038
> milliseconds
> 
> 

And with Safari on Mac I get: "Out of memory"

How is that? Does Safari have some control mechanism to limit the amount of memory a webview can use or something like that?

[Carlos Alberto Lopez Perez]

(In reply to Carlos Alberto Lopez Perez from comment #3)
> (In reply to Carlos Alberto Lopez Perez from comment #2)
> > Ok.. further investigation it seems the chrome results are not valid.
> > 
> > The try{} on the test was hidding that chrome was simply aborting with such
> > large string.
> > 
> > I have crafted a better test that will print either the time it takes to run
> > or the exception:
> > 
> > https://people.igalia.com/clopez/wkbug/largestrings/test-button.html
> > 
> > And I get:
> > 
> >  - Chrome: Invalid string length
> >  - Firefox: repeat count must be less than infinity and not overflow maximum
> > string size
> >  - WebKitGTK+: Test finished: total time to run: 1459.0900000000038
> > milliseconds
> > 
> > 
> 
> And with Safari on Mac I get: "Out of memory"
> 
> How is that? Does Safari have some control mechanism to limit the amount of
> memory a webview can use or something like that?

I guess this also explains why Safari peaks at 3GB and then lowers at 250MB.. it seems there is some kind of limit around 3GB.. meanwhile WebKitGTK+ is happy to use all the RAM available (and even more).

[Michael Catanzaro]

(In reply to Carlos Alberto Lopez Perez from comment #3) 
> And with Safari on Mac I get: "Out of memory"
> 
> How is that? Does Safari have some control mechanism to limit the amount of
> memory a webview can use or something like that?

Apple recently implemented a web process memory limit (I think 16 GiB, which is way too high IMO) at which point the web process crashes. We need that too (with a waaaaay lower limit), and it is already on our roadmap.

[Carlos Alberto Lopez Perez]

(In reply to Michael Catanzaro from comment #5)
> (In reply to Carlos Alberto Lopez Perez from comment #3) 
> > And with Safari on Mac I get: "Out of memory"
> > 
> > How is that? Does Safari have some control mechanism to limit the amount of
> > memory a webview can use or something like that?
> 
> Apple recently implemented a web process memory limit (I think 16 GiB, which
> is way too high IMO) at which point the web process crashes. We need that
> too (with a waaaaay lower limit), and it is already on our roadmap.

But note that the webprocess is not crashing here. its just javascript returning an exception "Out of memory" but no crashes. So its something different.

[Carlos Alberto Lopez Perez]

(In reply to Carlos Alberto Lopez Perez from comment #6)
> (In reply to Michael Catanzaro from comment #5)
> > (In reply to Carlos Alberto Lopez Perez from comment #3) 
> > > And with Safari on Mac I get: "Out of memory"
> > > 
> > > How is that? Does Safari have some control mechanism to limit the amount of
> > > memory a webview can use or something like that?
> > 
> > Apple recently implemented a web process memory limit (I think 16 GiB, which
> > is way too high IMO) at which point the web process crashes. We need that
> > too (with a waaaaay lower limit), and it is already on our roadmap.
> 
> But note that the webprocess is not crashing here. its just javascript
> returning an exception "Out of memory" but no crashes. So its something
> different.

It seems I got confused here because I was testing with my distro version of webkitgtk+ that is a bit old.

I'm also getting "Out of memory" exception with last trunk of WebKitGTK+, and also peaking at 3GB of RAM. And the memory is released after this peak.

So.. same behaviour on WebKitGTK+ than on Safari.

Its with a previous version of WebKitGTK+ (2.14.1) that I get the test to happily run.

[Carlos Alberto Lopez Perez]

So.. this is the issue:

	 function createRegexp() {
	     var s = "a".repeat(0x3fffffff);
	     var r = RegExp.prototype.toString.call({
		 source: s,
		 flags: s,
	     });
	     return [s, r];
	 };


Both Firefox and chrome abort at var s = "a".repeat(0x3fffffff); with "repeat count must be less than infinity and not overflow maximum string size" (firefox) or "Invalid string length".

But we happily build that monstrous string (which needs around 3GB of RAM!)

I created another test that demonstrates this: https://people.igalia.com/clopez/wkbug/largestrings/large-string-alive.html

Both with WebKitGTK+ and Safari if you click on the button you will see how WebKit uses around 3GB of RAM and doesn't discard it because the test is crafted to keep the string alive. You can actually click the button several times. Each time you click on the button the memory usage will grow by several GBs... You can guess it will be easy to create a malicious page with this that crashes the user's computer due to an OOM situation.


Firefox and Chrome refuse to build that string. I think we need to put also some limit on the string length.

[Carlos Alberto Lopez Perez]

(In reply to Carlos Alberto Lopez Perez from comment #8)
> So.. this is the issue:
> 
> 	 function createRegexp() {
> 	     var s = "a".repeat(0x3fffffff);
> 	     var r = RegExp.prototype.toString.call({
> 		 source: s,
> 		 flags: s,
> 	     });
> 	     return [s, r];
> 	 };
> 
> 
> Both Firefox and chrome abort at var s = "a".repeat(0x3fffffff); with
> "repeat count must be less than infinity and not overflow maximum string
> size" (firefox) or "Invalid string length".
> 

We abort later with "Out of memory" on the call to RegExp.prototype.toString() which such string. But that is already late for this test case. We should not allow to build such string in the very fist place.

[Yusuke Suzuki]

(In reply to Carlos Alberto Lopez Perez from comment #9)
> (In reply to Carlos Alberto Lopez Perez from comment #8)
> > So.. this is the issue:
> > 
> > 	 function createRegexp() {
> > 	     var s = "a".repeat(0x3fffffff);
> > 	     var r = RegExp.prototype.toString.call({
> > 		 source: s,
> > 		 flags: s,
> > 	     });
> > 	     return [s, r];
> > 	 };
> > 
> > 
> > Both Firefox and chrome abort at var s = "a".repeat(0x3fffffff); with
> > "repeat count must be less than infinity and not overflow maximum string
> > size" (firefox) or "Invalid string length".
> > 
> 
> We abort later with "Out of memory" on the call to
> RegExp.prototype.toString() which such string. But that is already late for
> this test case. We should not allow to build such string in the very fist
> place.

I'm not sure why the current behavior is undesirable.
We have an explicit limit of string length, INT32_MAX, so, the current behavior is expected.
If the length becomes larger than that limit, JSC will throw OOM error.

`"a".repeat(0x8fffffff)` => OOM error.

It seems that SpiderMonkey and V8 simply have lower limit of string length.

In the case of malicious page, even if the limit is lower, you can still easily create such a page if we retain the created strings.

var array = [];
for (var i = 0;; ++i) {
    array[i] = "a".repeat(0xffffff);
}

[Carlos Alberto Lopez Perez]

(In reply to Yusuke Suzuki from comment #10)
> (In reply to Carlos Alberto Lopez Perez from comment #9)
> > (In reply to Carlos Alberto Lopez Perez from comment #8)
> > > So.. this is the issue:
> > > 
> > > 	 function createRegexp() {
> > > 	     var s = "a".repeat(0x3fffffff);
> > > 	     var r = RegExp.prototype.toString.call({
> > > 		 source: s,
> > > 		 flags: s,
> > > 	     });
> > > 	     return [s, r];
> > > 	 };
> > > 
> > > 
> > > Both Firefox and chrome abort at var s = "a".repeat(0x3fffffff); with
> > > "repeat count must be less than infinity and not overflow maximum string
> > > size" (firefox) or "Invalid string length".
> > > 
> > 
> > We abort later with "Out of memory" on the call to
> > RegExp.prototype.toString() which such string. But that is already late for
> > this test case. We should not allow to build such string in the very fist
> > place.
> 
> I'm not sure why the current behavior is undesirable.
> We have an explicit limit of string length, INT32_MAX, so, the current
> behavior is expected.
> If the length becomes larger than that limit, JSC will throw OOM error.
> 
> `"a".repeat(0x8fffffff)` => OOM error.
> 
> It seems that SpiderMonkey and V8 simply have lower limit of string length.
> 
> In the case of malicious page, even if the limit is lower, you can still
> easily create such a page if we retain the created strings.
> 
> var array = [];
> for (var i = 0;; ++i) {
>     array[i] = "a".repeat(0xffffff);
> }


Here is a test for that:

https://people.igalia.com/clopez/wkbug/largestrings/large-array-alive.html

This is what I observer:

 * Chrome: there isn't any noticiable spike of RAM by clicking any of the buttons. You end getting and "Aw! snap" error page.. that is.. the webview is killed but without spiking in RAM usage. The user's running session will be fine.

 * Firefox: The worse situation by clicking any of the buttons is a speak of RAM around +3-4GB. Then it throws an "undefined" javascript exception. There is no crash of the webview. The user's running session will be fine if the computer can handle a spike of +3GB of RAM.

 * Safari/WebKitGTK+: It will use all your RAM and cause an OOM situation that can potentially crash not only the webview, but also the user's running session. I have to say that MacOS seems to deal much better with this. On Linux OOM situations like this usually cause the desktop to freeze for several minutes, which is a PITA.