Bug 192795 – Array unshift/shift should not race against the AI in the compiler thread.

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

[Mark Lam]

<rdar://problem/46724263>

[Mark Lam]

Created attachment 357527 [details]
proposed patch.

[Saam Barati]

Comment on attachment 357527 [details]
proposed patch.

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

> Source/JavaScriptCore/ChangeLog:16
> +        operations can move values around in the butterfly.  Hence, the fact that AI has

Do any operations do this without grabbing the lock? Or does this kind of moving around require holding the lock?

[Mark Lam]

Comment on attachment 357527 [details]
proposed patch.

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

Thanks for the review.

>> Source/JavaScriptCore/ChangeLog:16
>> +        operations can move values around in the butterfly.  Hence, the fact that AI has
> 
> Do any operations do this without grabbing the lock? Or does this kind of moving around require holding the lock?

I can't think of any other operations that keeps the moves the m_sparseMap around the butterfly like this.  I also did a survey of setButterfly(), nukeStructureAndSetButterfly(), the setting of arrayStorage()->m_indexBias, and the setting of arrayStorage()->m_sparseMap.  AFAICT, in all cases, we either
(1) allocating a new butterfly and fully initializing it before setting it in the object, which means just fetching the butterfly pointer is sufficient to guarantee we can read a meaningful value from it if the structure after is still the same, or
(2) the butterfly is already in ArrayStorage format, and we're setting the m_sparseMap either to a new sparse map or to null.  Other than unshift and shift, I didn't see anything that moves the location of m_sparseMap in the butterfly.  

So, I think we're good to go.

[WebKit Commit Bot]

Comment on attachment 357527 [details]
proposed patch.

Clearing flags on attachment: 357527

Committed r239325: <https://trac.webkit.org/changeset/239325>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.