Bug 164433

Summary:

REGRESSION: Crashes in StringImpl destructor during GC when clearing the HasOwnPropertyCache

Product:

WebKit

Reporter:

Jim Oase <jimoase>

Component:

JavaScriptCore

Assignee:

Saam Barati <saam>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ap, benjamin, fpizlo, ggaren, gskachkov, jfbastien, keith_miller, mark.lam, msaboff, oliver, rniwa, ryanhaddad, saam, ticaiolima, webkit-bug-importer, ysuzuki

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Crash log	none
patch	none

Jim Oase

Reported 2016-11-04 14:30:02 PDT

Every load results in a error message. The reload appears to work. http://www.video.theblaze.com/video/e187-274027 is the last site to fail

Attachments
Crash log (87.54 KB, text/plain) 2016-11-04 15:54 PDT, Jim Oase	no flags	Details
patch (1.82 KB, patch) 2016-11-07 15:24 PST, Saam Barati	no flags	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Jim Oase

Comment 1 2016-11-04 15:54:48 PDT

Created attachment 293945 [details] Crash log

Alexey Proskuryakov

Comment 2 2016-11-05 10:01:02 PDT

I couldn't reproduce this.

Alexey Proskuryakov

Comment 3 2016-11-05 15:15:41 PDT

But, this is seen on bots: https://build.webkit.org/results/Apple%20Sierra%20Release%20WK2%20(Tests)/r208418%20(1259)/imported/w3c/web-platform-tests/html/dom/interfaces-crash-log.txt

Alexey Proskuryakov

Comment 4 2016-11-05 15:22:12 PDT

First occurrence that I see was on 2016-11-02 16:21:01. Filip, could this be caused by threaded GC (r208306)?

Filip Pizlo

Comment 5 2016-11-05 15:30:00 PDT

(In reply to comment #4) > First occurrence that I see was on 2016-11-02 16:21:01. > > Filip, could this be caused by threaded GC (r208306)? Yup, that's the patch at fault. Should be really easy to fix. Basically, we just need to move anything in the GC that touches strings off the GC thread. It's usually easy to do this. Here we see the collector calling some HasOwnPropertyCache thing, which it shouldn't be doing.

Saam Barati

Comment 6 2016-11-07 15:08:34 PST

This looks like the HasOwnPropertyCache at work. It derefs StringImpls from the collector thread.

Saam Barati

Comment 7 2016-11-07 15:24:56 PST

Created attachment 294094 [details] patch

Mark Lam

Comment 8 2016-11-07 15:27:58 PST

Comment on attachment 294094 [details] patch r=me

Saam Barati

Comment 9 2016-11-07 16:49:42 PST

<rdar://problem/29079741>