com.apple.WebKit.Networking.Development crashes in WebCore::formOpen()

[Daniel Bates]

Crash reports show that com.apple.WebKit.Networking.Development can crash in WebCore::formOpen().

[Daniel Bates]

<rdar://problem/23550269>

[Daniel Bates]

Created attachment 272213 [details]
Patch

[Brent Fulgham]

Comment on attachment 272213 [details]
Patch

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

> Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:182
> +    LockHolder locker(form->streamIsBeingOpenedLock);

This seems fine, but we should be careful that we don't do any further locking in the code that follows (or the code that is called by the following). I'm not aware of any such case, but we should be on the lookout for deadlocks after this change lands.

[Daniel Bates]

(In reply to comment #3)
> Comment on attachment 272213 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=272213&action=review
> 
> > Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:182
> > +    LockHolder locker(form->streamIsBeingOpenedLock);
> 
> This seems fine, but we should be careful that we don't do any further
> locking in the code that follows 

Care will need to be taken when additional locks are used in this code to prevent deadlock/livelock.

> (or the code that is called by the following).

I will update the patch to assert that the added lock is held in WebCore::advanceCurrentStream() and WebCore::closeCurrentStream() to catch calls to these functions that are made without holding the lock.

[Daniel Bates]

Created attachment 272571 [details]
Patch

[Brent Fulgham]

Comment on attachment 272571 [details]
Patch

r=me. If EWS is green (ignore Windows right now) please land.

[Daniel Bates]

Comment on attachment 272571 [details]
Patch

Clearing flags on attachment: 272571

Committed r197424: <http://trac.webkit.org/changeset/197424>

[Daniel Bates]

All reviewed patches have been landed.  Closing bug.