Bug 180323 – Add test demonstrating leaks that happen when we create reference cycles with DOM objects

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Darin Adler]

Add test demonstrating leaks that happen when we create reference cycles with DOM Objects

[Darin Adler]

Created attachment 328266 [details]
Patch

[EWS Watchlist]

Comment on attachment 328266 [details]
Patch

Attachment 328266 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/5471032

New failing tests:
fast/dom/reference-cycle-leaks.html

[EWS Watchlist]

Created attachment 328270 [details]
Archive of layout-test-results from ews101 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[EWS Watchlist]

Comment on attachment 328266 [details]
Patch

Attachment 328266 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/5471049

New failing tests:
fast/dom/reference-cycle-leaks.html

[EWS Watchlist]

Created attachment 328271 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Darin Adler]

I guessed that maybe the test failures were due to the builds being Release instead of Debug, so I made a local release build. But I could not reproduce those failures; works consistently here. Another difference is that I am testing on High Sierra, not El Capitan, two major macOS releases newer.

I can think of two options right away if I can’t figure out why the EWS tests are failing:

1) I could decide that this testing strategy is intrinsically too flaky to add these tests. So just not add these.

2) I could remove or at least temporarily disable the tests that are behaving inconsistently: createTreeWalkerNodeCycle, createTreeWalkerFilterCycle, and createPromiseCycle.

Any other ideas?

[Geoffrey Garen]

You might be able to make the test more reliable by making the action -> GC link async. That way, the GC happens on a clean stack, reducing the chances of floating garbage caused by conservative GC.

A complimentary option is to expose JSC::sanitizeStackForVM() explicitly through window.internals, and incorporate it into an async GC chain: action -> sanitize -> GC.

Another option is to allocate lots of objects and allow a small number of survivors to count as a pass. This option is preferable to just removing tests because it still catches deterministic leaks.

[Filip Pizlo]

If I want to prove that the GC is capable of freeing a kind of object, I like to allocate ~10000 of them and say that the test passes if *any* of them got freed. 

This side steps the problem that the GC may keep any object alive with some small probability. It may keep one of them alive but it won’t keep all of them alive in that case.

[Darin Adler]

(In reply to Filip Pizlo from comment #8)
> If I want to prove that the GC is capable of freeing a kind of object, I
> like to allocate ~10000 of them and say that the test passes if *any* of
> them got freed. 

I’ll try changing this to use that technique to see what happens.

[Darin Adler]

I see now that this was Geoff’s suggestion #3 too.

Hope I don’t need something more serious like #1 or #2.

[Darin Adler]

Created attachment 328919 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 328919 [details]
Patch

Clearing flags on attachment: 328919

Committed r225729: <https://trac.webkit.org/changeset/225729>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/35954395>