Title: [197424] trunk/Source/WebCore
- Revision
- 197424
- Author
- [email protected]
- Date
- 2016-03-01 16:01:31 -0800 (Tue, 01 Mar 2016)
Log Message
com.apple.WebKit.Networking.Development crashes in WebCore::formOpen()
https://bugs.webkit.org/show_bug.cgi?id=154682
<rdar://problem/23550269>
Reviewed by Brent Fulgham.
Speculative fix for a race condition when opening the stream for the next form data element.
Calling CFReadStreamOpen(s) in WebCore::openNextStream() can cause stream s to be closed and
deallocated before CFReadStreamOpen(s) returns.
When WebCore::openNextStream() is called it closes and deallocates the current stream and
then opens a new stream for the next form data element. Calling CFReadStreamOpen() in
WebCore::openNextStream() can lead to WebCore::openNextStream() being re-entered via
WebCore::formEventCallback() from another thread. One example when this can occur is when
the stream being opened has no data (i.e. WebCore::formEventCallback() is called
back with event type kCFStreamEventEndEncountered).
I have been unable to reproduce this crash. We know that it occurs from crash reports.
* platform/network/cf/FormDataStreamCFNet.cpp:
(WebCore::closeCurrentStream): Assert that we had acquired a lock to close the stream.
(WebCore::advanceCurrentStream): Assert that we had acquired a lock to advance the stream.
(WebCore::openNextStream): Acquire a lock before we open the next stream to ensure that
exactly one thread executes this critical section at a time.
(WebCore::formFinalize): Acquire a lock before we close the current stream.
(WebCore::formClose): Ditto.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (197423 => 197424)
--- trunk/Source/WebCore/ChangeLog 2016-03-02 00:00:02 UTC (rev 197423)
+++ trunk/Source/WebCore/ChangeLog 2016-03-02 00:01:31 UTC (rev 197424)
@@ -1,3 +1,32 @@
+2016-03-01 Daniel Bates <[email protected]>
+
+ com.apple.WebKit.Networking.Development crashes in WebCore::formOpen()
+ https://bugs.webkit.org/show_bug.cgi?id=154682
+ <rdar://problem/23550269>
+
+ Reviewed by Brent Fulgham.
+
+ Speculative fix for a race condition when opening the stream for the next form data element.
+ Calling CFReadStreamOpen(s) in WebCore::openNextStream() can cause stream s to be closed and
+ deallocated before CFReadStreamOpen(s) returns.
+
+ When WebCore::openNextStream() is called it closes and deallocates the current stream and
+ then opens a new stream for the next form data element. Calling CFReadStreamOpen() in
+ WebCore::openNextStream() can lead to WebCore::openNextStream() being re-entered via
+ WebCore::formEventCallback() from another thread. One example when this can occur is when
+ the stream being opened has no data (i.e. WebCore::formEventCallback() is called
+ back with event type kCFStreamEventEndEncountered).
+
+ I have been unable to reproduce this crash. We know that it occurs from crash reports.
+
+ * platform/network/cf/FormDataStreamCFNet.cpp:
+ (WebCore::closeCurrentStream): Assert that we had acquired a lock to close the stream.
+ (WebCore::advanceCurrentStream): Assert that we had acquired a lock to advance the stream.
+ (WebCore::openNextStream): Acquire a lock before we open the next stream to ensure that
+ exactly one thread executes this critical section at a time.
+ (WebCore::formFinalize): Acquire a lock before we close the current stream.
+ (WebCore::formClose): Ditto.
+
2016-03-01 Michael Saboff <[email protected]>
ASSERT in platform/graphics/mac/ComplexTextController.cpp::capitalize()
Modified: trunk/Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp (197423 => 197424)
--- trunk/Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp 2016-03-02 00:00:02 UTC (rev 197423)
+++ trunk/Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp 2016-03-02 00:01:31 UTC (rev 197424)
@@ -109,10 +109,13 @@
CFReadStreamRef formStream;
unsigned long long streamLength;
unsigned long long bytesSent;
+ Lock streamIsBeingOpenedOrClosedLock;
};
static void closeCurrentStream(FormStreamFields* form)
{
+ ASSERT(form->streamIsBeingOpenedOrClosedLock.isHeld());
+
if (form->currentStream) {
CFReadStreamClose(form->currentStream);
CFReadStreamSetClient(form->currentStream, kCFStreamEventNone, 0, 0);
@@ -127,6 +130,8 @@
// Return false if we cannot advance the stream. Currently the only possible failure is that the underlying file has been removed or changed since File.slice.
static bool advanceCurrentStream(FormStreamFields* form)
{
+ ASSERT(form->streamIsBeingOpenedOrClosedLock.isHeld());
+
closeCurrentStream(form);
if (form->remainingElements.isEmpty())
@@ -176,6 +181,10 @@
static bool openNextStream(FormStreamFields* form)
{
+ // CFReadStreamOpen() can cause this function to be re-entered from another thread before it returns.
+ // One example when this can occur is when the stream being opened has no data. See <rdar://problem/23550269>.
+ LockHolder locker(form->streamIsBeingOpenedOrClosedLock);
+
// Skip over any streams we can't open.
if (!advanceCurrentStream(form))
return false;
@@ -213,7 +222,10 @@
ASSERT_UNUSED(stream, form->formStream == stream);
callOnMainThread([form] {
- closeCurrentStream(form);
+ {
+ LockHolder locker(form->streamIsBeingOpenedOrClosedLock);
+ closeCurrentStream(form);
+ }
delete form;
});
}
@@ -281,6 +293,7 @@
static void formClose(CFReadStreamRef, void* context)
{
FormStreamFields* form = static_cast<FormStreamFields*>(context);
+ LockHolder locker(form->streamIsBeingOpenedOrClosedLock);
closeCurrentStream(form);
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
