Bug 22869 – REGRESSION (r38407): http://news.cnet.com/8301-13579_3-9953533-37.html crashes

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

[Cameron Zwarich (cpst)]

This was split off from bug 22797, because it is a separate issue at the same URL. The original crash was due to a bug in plugin code. However, if I reload this page repeatedly, I see another crash even with plugins disabled.

I am going to try to narrow down the exact revision that caused this, because reducing it doesn't seem to be working out that well.

[Cameron Zwarich (cpst)]

<rdar://problem/6402499>

[Cameron Zwarich (cpst)]

I have tracked this down to the range between the r38386 and r38492 nightlies.

[Cameron Zwarich (cpst)]

This regressed in r38407:

http://trac.webkit.org/changeset/38407

[Cameron Zwarich (cpst)]

Created attachment 26039 [details]
Stack trace

This will crash with a lot of different stack traces, depending on the revision and build type, but I get this one pretty consistently on a local debug build of r38407 with MallocScribble enabled.

[Cameron Zwarich (cpst)]

I am having a lot of trouble with this.

[Cameron Zwarich (cpst)]

This is fixed by making Structure::m_nameInPrevious a RefPtr. Does anyone know why it isn't currently a RefPtr?

[Cameron Zwarich (cpst)]

I understand why this is a problem. Previously, m_nameInPrevious was being ref'd because it was put into the PropertyMap. Now, the PropertyMap is lazily created, so m_nameInPrevious is not ref'd until the PropertyMap is created.

It would presumedly be possible to catch this with an assertion on UString::Rep::ref(), but there are a few places where ref counts of UString::Reps are deliberately zeroed out, so the assertion won't work until those are fixed.

[Cameron Zwarich (cpst)]

Created attachment 26049 [details]
Proposed patch

[Darin Adler]

Comment on attachment 26049 [details]
Proposed patch

I'm really disappointed that we can't regression test this. I'd like to know more about the "few places where ref counts of UString::Reps are deliberately zeroed out" too.

r=me

[Cameron Zwarich (cpst)]

Fixed in r39431. Darin, I'll make another bug about fixing UString ref counting.