Bug 147403 – web process continually eating memory on simple, shared Google Docs spreadsheet

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Matt Lilek]

Created attachment 257739 [details]
Really simple sheet example

Loading a simple, shared Google Docs spreadsheet and leaving it open causes the web process to steadily consume more and more memory with only a small amount of leaks showing up in the `leaks` tool. Encountered this today after leaving a stupidly simple shared spreadsheet open in a tab on my work laptop.

Filing here instead of Radar for easier back & forth since, unfortunately, I can't seem to get a ton of good data on what's actually happening here. The `leaks` tool tells me that the web process is marked CS_RESTRICT so I'm not sure if it's giving me everything and the Allocations instrument won't attach, presumably for the same reason.

I'm not sure whether the steps that add content are necessary to reproduce this, but ¯\_(ツ)_/¯.

* STEPS TO REPRODUCE
1. Open https://docs.google.com/spreadsheets
2. Click the (+) in the bottom right corner to create a new, blank spreadsheet.
3. Add some simple text on the first couple of cells in the first column.
4. Create a second sheet using the + button in the lower left corner and add similar content to the first sheet.
5. Share the document with a couple of other people.
6. In another browser or on another machine, open the shared sheet from a different account. Repeat this twice.

After just a few minutes, the memory use went from ~350 MB when the page was initially loaded to ~700 MB. While not the most accurate and useful info and assuming leaks can be trusted on my machine, this is the kind of growth I was seeing with the above repro steps, just letting it sit there, once or twice adding a new line of content to the sheet (i.e.: between the last two entries below):

Launch Time:     2015-07-28 22:10:48.901 -0700
Date/Time:  2015-07-28 22:11:16.432 -0700    Process 1130: 26305 nodes malloced for 121329 KB
Date/Time:  2015-07-28 22:11:31.692 -0700    Process 1130: 66790 nodes malloced for 404615 KB
Date/Time:  2015-07-28 22:12:42.172 -0700    Process 1130: 66071 nodes malloced for 495008 KB
Date/Time:  2015-07-28 22:16:11.218 -0700    Process 1130: 67538 nodes malloced for 740611 KB
Date/Time:  2015-07-28 22:19:10.272 -0700    Process 1130: 71327 nodes malloced for 954714 KB
Date/Time:  2015-07-28 22:33:30.477 -0700    Process 1130: 76714 nodes malloced for 1873688 KB
Date/Time:  2015-07-28 22:38:46.192 -0700    Process 1130: 78128 nodes malloced for 2561626 KB
Date/Time:  2015-07-28 22:49:17.383 -0700    Process 1130: 73528 nodes malloced for 2616807 KB
Date/Time:  2015-07-28 22:56:17.250 -0700    Process 1130: 85116 nodes malloced for 3177611 KB

When I noticed this on my machine earlier today, activity monitor claimed the web process was eating 4.3GB of memory. That was on 10.10.5 (14F6a) on a 2015, 13" Retina MBP with Safari 9.0 (10601.1.39.0.2) and tonight I reproduced using the steps above on a 2013 iMac running 10.11 (15A235d) with Safari 9.0 (11601.1.41). The other two accounts were on the same iMac in Chrome 44.0.2403.89 which didn't have any noticeable memory growth over the same time.

FWIW, `leaks` does show a number of small RTCReporting leaks for a grand total of 7360 leaked bytes.

[Matt Lilek]

Created attachment 257740 [details]
Leaks

[Andreas Kling]

<rdar://problem/18835799>

[Andreas Kling]

This appears to happen because the JSC garbage collector expects to be able to schedule work using a CFRunLoopTimer, but when executing inside a web worker thread, there is no CFRunLoop to fire those timers.

I'm investigating some different solutions.

[Darin Adler]

Alexey says there is some class we have that uses the run loop on the main thread and does something else on other threads that we could possibly use.

[Alexey Proskuryakov]

I was thinking about threadGlobalData().threadTimers(), which has an instance of SharedTimer class that is appropriate for the thread. This is what makes DOM timers function in web workers.

But this code is in WebCore.

[Andreas Kling]

Created attachment 257867 [details]
Patch

This patch fixes the problem by having WorkerRunLoop service the CFRunLoop. It tries to avoid waking up until there's a CFRunLoop timer that would fire, though it doesn't try *that* hard; WebCore timers will also wake it up, since they sit on the same code path.

This is a stopgap fix for ports using CoreFoundation. Other ports that aren't using CF may not have this problem in the first place. I will revisit this down the line and work out a better design where JSC::HeapTimer doesn't muck around directly with the CFRunLoop.

[Andreas Kling]

Created attachment 257875 [details]
Patch

Gavin pointed out that it would be excessive to service the CFRunLoop every time a 1000hz DOM timer fires.
This patch makes sure to only call CFRunLoopRunInMode if the next CFRunLoop timer wants to fire now or in the past.

[Geoffrey Garen]

Comment on attachment 257875 [details]
Patch

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

r=me

> Source/WebCore/workers/WorkerRunLoop.cpp:184
> +        if (nextCFRunLoopTimerFireDate <= CFAbsoluteTimeGetCurrent())
> +            CFRunLoopRunInMode(kCFRunLoopDefaultMode, 0, true);

I think we want returnAfterSourceHandled to be false (and also in a local variable so the next poor soul who reads this code knows what it means). That way, if more than one source is pending, it will service right away.

[Andreas Kling]

Created attachment 257880 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 257880 [details]
Patch for landing

Clearing flags on attachment: 257880

Committed r187630: <http://trac.webkit.org/changeset/187630>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Alexis Menard (darktears)]

Safari 9.1.3 still leaks badly when opening a Google Drive/Doc document, using the same repro steps of comment 1.

Apparently various people have the same problem :
https://discussions.apple.com/thread/6640430?start=0&tstart=0
https://productforums.google.com/forum/#!topic/docs/nLs2qqq-9os
https://discussions.apple.com/thread/6857911?start=0&tstart=0
https://www.reddit.com/r/google/comments/30lzym/i_think_weve_got_a_memory_leak_guys/

This comment seems to provide a workaround, not sure that's legitimate :
https://forums.developer.apple.com/thread/8497

Firefox (48.0.2) and Chrome (53.0.2785.101) does not exhibit the behavior.