Changeset 198335 in webkit

Timestamp:

Mar 17, 2016, 10:02:14 AM (9 years ago)

Author:

Antti Koivisto

Message:

DataURLDecoder::DecodingResultDispatcher may get deleted outside main thread
​https://bugs.webkit.org/show_bug.cgi?id=155584
rdar://problem/24492104

Reviewed by Chris Dumez.

This is unsafe as it owns strings and other types that are only safe to delete in the main thread.

• platform/network/DataURLDecoder.cpp:

(WebCore::DataURLDecoder::DecodingResultDispatcher::dispatch):

The problem is that this was a refcounted type. This created a race. If the timer fired before dispatch()
was exited the implicit deref here would trigger the deletion in the dispatching thread.

Fix by getting rid of the unnecessary refcounting. Timer firing will now delete the instance explicitly.

(WebCore::DataURLDecoder::DecodingResultDispatcher::startTimer):
(WebCore::DataURLDecoder::DecodingResultDispatcher::timerFired):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/network/DataURLDecoder.cpp (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-03-17  Antti Koivisto  <antti@apple.com>
+
+        DataURLDecoder::DecodingResultDispatcher may get deleted outside main thread
+        https://bugs.webkit.org/show_bug.cgi?id=155584
+        rdar://problem/24492104
+
+        Reviewed by Chris Dumez.
+
+        This is unsafe as it owns strings and other types that are only safe to delete in the main thread.
+
+        * platform/network/DataURLDecoder.cpp:
+        (WebCore::DataURLDecoder::DecodingResultDispatcher::dispatch):
+
+            The problem is that this was a refcounted type. This created a race. If the timer fired before dispatch()
+            was exited the implicit deref here would trigger the deletion in the dispatching thread.
+
+            Fix by getting rid of the unnecessary refcounting. Timer firing will now delete the instance explicitly.
+
+        (WebCore::DataURLDecoder::DecodingResultDispatcher::startTimer):
+        (WebCore::DataURLDecoder::DecodingResultDispatcher::timerFired):
+
 2016-03-17  Commit Queue  <commit-queue@webkit.org>
 

trunk/Source/WebCore/platform/network/DataURLDecoder.cpp

@@ -57 +57 @@
 #if HAVE(RUNLOOP_TIMER)
 
-class DecodingResultDispatcher : public ThreadSafeRefCounted<DecodingResultDispatcher> {
+class DecodingResultDispatcher {
+    WTF_MAKE_FAST_ALLOCATED;
 public:
     static void dispatch(std::unique_ptr<DecodeTask> decodeTask)
     {
-        Ref<DecodingResultDispatcher> dispatcher = adoptRef(*new DecodingResultDispatcher(WTFMove(decodeTask)));
+        auto* dispatcher = new DecodingResultDispatcher(WTFMove(decodeTask));
         dispatcher->startTimer();
     }
@@ -74 +75 @@
     void startTimer()
     {
-        // Keep alive until the timer has fired.
-        ref();
         m_timer.startOneShot(0);
         m_timer.schedule(m_decodeTask->scheduleContext.scheduledPairs);
@@ -87 +86 @@
             m_decodeTask->completionHandler({ });
 
-        deref();
+        delete this;
     }