Bug 154146 – AdaptiveInferredPropertyValueWatchpoint can trigger a GC that frees its CodeBlock and thus itself

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

[Keith Miller]

Consider the following: there is some CodeBlock, C, that is watching some object, O, with a structure, S, for replacements. Also, suppose that C has no references anymore and is due to be GCed. Now, when some new property is added to O, S will create a new structure S' and fire its transition watchpoints. Since C is watching S for replacements it will attempt to have its AdaptiveInferredPropertyValueWatchpoint relocate itself to S'. To do so, it needs it allocate RareData on S'. This allocation may cause a GC, which frees C while still executing its watchpoint handler.

[Keith Miller]

Created attachment 271117 [details]
Patch

[Keith Miller]

I can't find a good way to test this. The main issue is that we need to take a slow path allocation in order to trigger a GC but we currently don't have a way to force allocation slow paths. I tried adding an option but the check for the option caused performance regressions. We could add it in debug builds only but that has the downside of making the test only effective in debug builds. Additionally, I have not found a way to get a CodeBlock to become unreferenced consistently, which makes the other issues somewhat moot.

[Keith Miller]

rdar://problem/23569888

[Keith Miller]

Note, I am confident this fixes the issue as previously we would crash reliably every run of "run-webkit-tests --additional-env-var="JSC_slowPathAllocsBetweenGCs=10" --no-retry -1 --child-processes=6 -g svg/dom/viewspec-parser-4.html --repeat 100" and with the change I have not crashed in 1000+ runs.

[WebKit Commit Bot]

Comment on attachment 271117 [details]
Patch

Clearing flags on attachment: 271117

Committed r196497: <http://trac.webkit.org/changeset/196497>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.