Bug 154682

Summary: com.apple.WebKit.Networking.Development crashes in WebCore::formOpen()
Product: WebKit Reporter: Daniel Bates <dbates>
Component: PlatformAssignee: Daniel Bates <dbates>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, bfulgham, darin, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Mac   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Daniel Bates
Reported 2016-02-25 10:04:08 PST
Crash reports show that com.apple.WebKit.Networking.Development can crash in WebCore::formOpen().
Attachments
Patch (2.83 KB, patch)
2016-02-25 10:22 PST, Daniel Bates 		no flags
DetailsFormatted DiffDiff
Patch (4.48 KB, patch)
2016-03-01 10:16 PST, Daniel Bates 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Daniel Bates
Comment 1 2016-02-25 10:04:29 PST
<rdar://problem/23550269>
Daniel Bates
Comment 2 2016-02-25 10:22:24 PST
Created attachment 272213 [details] Patch
Brent Fulgham
Comment 3 2016-02-25 10:47:11 PST
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
Comment 4 2016-03-01 10:13:24 PST
(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
Comment 5 2016-03-01 10:16:52 PST
Created attachment 272571 [details] Patch
Brent Fulgham
Comment 6 2016-03-01 10:24:14 PST
Comment on attachment 272571 [details] Patch r=me. If EWS is green (ignore Windows right now) please land.
Daniel Bates
Comment 7 2016-03-01 16:01:23 PST
Comment on attachment 272571 [details] Patch Clearing flags on attachment: 272571 Committed r197424: <http://trac.webkit.org/changeset/197424>
Daniel Bates
Comment 8 2016-03-01 16:01:27 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.