Title: [101813] trunk/Source/WebCore
Revision
101813
Author
da...@apple.com
Date
2011-12-02 09:42:54 -0800 (Fri, 02 Dec 2011)

Log Message

[Mac] Reference count threading violation in FormDataStreamMac.mm
https://bugs.webkit.org/show_bug.cgi?id=73627

Reviewed by Sam Weinig.

Shows up as a crash during existing layout test runs so no new tests are required.

* platform/network/mac/FormDataStreamMac.mm:
(WebCore::streamFieldsMap): Replaced getStreamFormDataMap with this.
Use an NSMapTable instead of a HashMap because we need to remove items from this
on a non-main thread.
(WebCore::associateStreamWithResourceHandle): Use NSMapGet instead of
HashMap::contains here.
(WebCore::formCreate): FormStreamFields now stores a RefPtr to the form data.
Added the code to fill that in. Did it in a more modern way to avoid the leakRef
and adoptRef that were used before. Replaced the code that set up the stream
form data map entry with code that sets an entry in the streamFieldsMap.
(WebCore::formFinishFinalizationOnMainThread): Added. Contains the work of
finalization that must be done on the main thread, specifically, destroying the
fields structure that contains objects with RefPtr in them. We can't touch these
reference counts on non-main threads.
(WebCore::formFinalize): Changed this to use NSMapRemove on the streamFieldsMap.
Added a callOnMainThread to finish the finalization.
(WebCore::setHTTPBody): Removed the leakRef, no longer needed, that used to be
balanced by an adoptRef in formCreate.
(WebCore::httpBodyFromStream): Changed to use NSMapGet.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (101812 => 101813)


--- trunk/Source/WebCore/ChangeLog	2011-12-02 17:42:04 UTC (rev 101812)
+++ trunk/Source/WebCore/ChangeLog	2011-12-02 17:42:54 UTC (rev 101813)
@@ -1,3 +1,32 @@
+2011-12-01  Darin Adler  <da...@apple.com>
+
+        [Mac] Reference count threading violation in FormDataStreamMac.mm
+        https://bugs.webkit.org/show_bug.cgi?id=73627
+
+        Reviewed by Sam Weinig.
+
+        Shows up as a crash during existing layout test runs so no new tests are required.
+
+        * platform/network/mac/FormDataStreamMac.mm:
+        (WebCore::streamFieldsMap): Replaced getStreamFormDataMap with this.
+        Use an NSMapTable instead of a HashMap because we need to remove items from this
+        on a non-main thread.
+        (WebCore::associateStreamWithResourceHandle): Use NSMapGet instead of
+        HashMap::contains here.
+        (WebCore::formCreate): FormStreamFields now stores a RefPtr to the form data.
+        Added the code to fill that in. Did it in a more modern way to avoid the leakRef
+        and adoptRef that were used before. Replaced the code that set up the stream
+        form data map entry with code that sets an entry in the streamFieldsMap.
+        (WebCore::formFinishFinalizationOnMainThread): Added. Contains the work of
+        finalization that must be done on the main thread, specifically, destroying the
+        fields structure that contains objects with RefPtr in them. We can't touch these
+        reference counts on non-main threads.
+        (WebCore::formFinalize): Changed this to use NSMapRemove on the streamFieldsMap.
+        Added a callOnMainThread to finish the finalization.
+        (WebCore::setHTTPBody): Removed the leakRef, no longer needed, that used to be
+        balanced by an adoptRef in formCreate.
+        (WebCore::httpBodyFromStream): Changed to use NSMapGet.
+
 2011-12-02  Antti Koivisto  <an...@apple.com>
 
         https://bugs.webkit.org/show_bug.cgi?id=73520

Modified: trunk/Source/WebCore/platform/network/mac/FormDataStreamMac.mm (101812 => 101813)


--- trunk/Source/WebCore/platform/network/mac/FormDataStreamMac.mm	2011-12-02 17:42:04 UTC (rev 101812)
+++ trunk/Source/WebCore/platform/network/mac/FormDataStreamMac.mm	2011-12-02 17:42:54 UTC (rev 101813)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2005, 2006, 2008 Apple Inc.  All rights reserved.
+ * Copyright (C) 2005, 2006, 2008, 2011 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -56,11 +56,11 @@
 
 namespace WebCore {
 
-typedef HashMap<CFReadStreamRef, RefPtr<FormData> > StreamFormDataMap;
-static StreamFormDataMap& getStreamFormDataMap()
+// We need to use NSMapTable instead of HashMap since this needs to be accessed from more than one thread.
+static NSMapTable *streamFieldsMap()
 {
-    DEFINE_STATIC_LOCAL(StreamFormDataMap, streamFormDataMap, ());
-    return streamFormDataMap;
+    static NSMapTable *streamFieldsMap = NSCreateMapTable(NSNonRetainedObjectMapKeyCallBacks, NSNonOwnedPointerMapValueCallBacks, 1);
+    return streamFieldsMap;
 }
 
 typedef HashMap<CFReadStreamRef, RefPtr<ResourceHandle> > StreamResourceHandleMap;
@@ -79,7 +79,7 @@
     if (!stream)
         return;
 
-    if (!getStreamFormDataMap().contains((CFReadStreamRef)stream))
+    if (!NSMapGet(streamFieldsMap(), stream))
         return;
 
     ASSERT(!getStreamResourceHandleMap().contains((CFReadStreamRef)stream));
@@ -125,11 +125,12 @@
 static void formEventCallback(CFReadStreamRef stream, CFStreamEventType type, void* context);
 
 struct FormContext {
-    FormData* formData;
+    RefPtr<FormData> formData;
     unsigned long long streamLength;
 };
 
 struct FormStreamFields {
+    RefPtr<FormData> formData;
     SchedulePairHashSet scheduledRunLoopPairs;
     Vector<FormDataElement> remainingElements; // in reverse order
     CFReadStreamRef currentStream;
@@ -230,6 +231,7 @@
     FormContext* formContext = static_cast<FormContext*>(context);
 
     FormStreamFields* newInfo = new FormStreamFields;
+    newInfo->formData = formContext->formData.release();
     newInfo->currentStream = NULL;
 #if ENABLE(BLOB)
     newInfo->currentStreamRangeLength = BlobDataItem::toEndOfFile;
@@ -239,28 +241,41 @@
     newInfo->streamLength = formContext->streamLength;
     newInfo->bytesSent = 0;
 
-    FormData* formData = formContext->formData;
-
     // Append in reverse order since we remove elements from the end.
-    size_t size = formData->elements().size();
+    size_t size = newInfo->formData->elements().size();
     newInfo->remainingElements.reserveInitialCapacity(size);
     for (size_t i = 0; i < size; ++i)
-        newInfo->remainingElements.append(formData->elements()[size - i - 1]);
+        newInfo->remainingElements.append(newInfo->formData->elements()[size - i - 1]);
 
-    getStreamFormDataMap().set(stream, adoptRef(formData));
+    ASSERT(!NSMapGet(streamFieldsMap(), stream));
+    NSMapInsertKnownAbsent(streamFieldsMap(), stream, newInfo);
 
     return newInfo;
 }
 
-static void formFinalize(CFReadStreamRef stream, void* context)
+static void formFinishFinalizationOnMainThread(void* context)
 {
     OwnPtr<FormStreamFields> form = adoptPtr(static_cast<FormStreamFields*>(context));
 
-    getStreamFormDataMap().remove(stream);
-
     closeCurrentStream(form.get());
 }
 
+static void formFinalize(CFReadStreamRef stream, void* context)
+{
+    FormStreamFields* form = static_cast<FormStreamFields*>(context);
+
+    ASSERT(form->formStream == stream);
+    ASSERT(NSMapGet(streamFieldsMap(), stream) == context);
+
+    // Do this right away because the CFReadStreamRef is being deallocated.
+    // We can't wait to remove this from the map until we finish finalizing
+    // on the main thread because in theory the freed memory could be reused
+    // for a new CFReadStream before that runs.
+    NSMapRemove(streamFieldsMap(), stream);
+
+    callOnMainThread(formFinishFinalizationOnMainThread, form);
+}
+
 static Boolean formOpen(CFReadStreamRef, CFStreamError* error, Boolean* openComplete, void* context)
 {
     FormStreamFields* form = static_cast<FormStreamFields*>(context);
@@ -471,7 +486,7 @@
     // Create and set the stream.
 
     // Pass the length along with the formData so it does not have to be recomputed.
-    FormContext formContext = { formData.release().leakRef(), length };
+    FormContext formContext = { formData.release(), length };
 
     RetainPtr<CFReadStreamRef> stream(AdoptCF, wkCreateCustomCFReadStream(formCreate, formFinalize,
         formOpen, formRead, formCanRead, formClose, formSchedule, formUnschedule,
@@ -481,7 +496,10 @@
 
 FormData* httpBodyFromStream(NSInputStream* stream)
 {
-    return getStreamFormDataMap().get((CFReadStreamRef)stream).get();
+    FormStreamFields* formStream = static_cast<FormStreamFields*>(NSMapGet(streamFieldsMap(), stream));
+    if (!formStream)
+        return 0;
+    return formStream->formData.get();
 }
 
 } // namespace WebCore
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to