[webkit-changes] [153488] trunk/Source/WebKit2

Tue, 30 Jul 2013 12:44:16 -0700

Title: [153488] trunk/Source/WebKit2
Revision
153488
Author
[email protected]
Date
2013-07-30 12:45:09 -0700 (Tue, 30 Jul 2013)

Log Message

Plug-in process crashes if plug-in is destroyed as a result of sending NPObjectMessageReceiver::Deallocate
https://bugs.webkit.org/show_bug.cgi?id=119270
<rdar://problem/13368226>

Reviewed by Darin Adler.

Normally we use the PluginDestructionProtector RAII object to prevent plug-ins from being destroyed while
they're executing code. However, in the case of the NPObjectMessageReceiver::Deallocate message, we can't do this
since we don't know the plug-in or connection.

Instead, add a counter to Connection that keeps track of whether sendSync is currently called and defer plug-in
destruction if it is. (This approach is actually more robust and we should investigate getting rid of the destruction protector).

* Platform/CoreIPC/Connection.cpp:
(CoreIPC::Connection::Connection):
(CoreIPC::Connection::sendSyncMessage):
* Platform/CoreIPC/Connection.h:
(CoreIPC::Connection::inSendSync):
* PluginProcess/PluginControllerProxy.cpp:
(WebKit::PluginControllerProxy::destroy):

Modified Paths

Diff

Modified: trunk/Source/WebKit2/ChangeLog (153487 => 153488)


--- trunk/Source/WebKit2/ChangeLog	2013-07-30 19:42:31 UTC (rev 153487)
+++ trunk/Source/WebKit2/ChangeLog	2013-07-30 19:45:09 UTC (rev 153488)
@@ -1,3 +1,26 @@
+2013-07-30  Anders Carlsson  <[email protected]>
+
+        Plug-in process crashes if plug-in is destroyed as a result of sending NPObjectMessageReceiver::Deallocate
+        https://bugs.webkit.org/show_bug.cgi?id=119270
+        <rdar://problem/13368226>
+
+        Reviewed by Darin Adler.
+
+        Normally we use the PluginDestructionProtector RAII object to prevent plug-ins from being destroyed while
+        they're executing code. However, in the case of the NPObjectMessageReceiver::Deallocate message, we can't do this
+        since we don't know the plug-in or connection. 
+        
+        Instead, add a counter to Connection that keeps track of whether sendSync is currently called and defer plug-in
+        destruction if it is. (This approach is actually more robust and we should investigate getting rid of the destruction protector).
+
+        * Platform/CoreIPC/Connection.cpp:
+        (CoreIPC::Connection::Connection):
+        (CoreIPC::Connection::sendSyncMessage):
+        * Platform/CoreIPC/Connection.h:
+        (CoreIPC::Connection::inSendSync):
+        * PluginProcess/PluginControllerProxy.cpp:
+        (WebKit::PluginControllerProxy::destroy):
+
 2013-07-30  Tim Horton  <[email protected]>
 
         Null deref under WebPage::scaledSnapshotWithOptions

Modified: trunk/Source/WebKit2/Platform/CoreIPC/Connection.cpp (153487 => 153488)


--- trunk/Source/WebKit2/Platform/CoreIPC/Connection.cpp	2013-07-30 19:42:31 UTC (rev 153487)
+++ trunk/Source/WebKit2/Platform/CoreIPC/Connection.cpp	2013-07-30 19:45:09 UTC (rev 153488)
@@ -225,6 +225,7 @@
     , m_isConnected(false)
     , m_connectionQueue(WorkQueue::create("com.apple.CoreIPC.ReceiveQueue"))
     , m_clientRunLoop(clientRunLoop)
+    , m_inSendSyncCount(0)
     , m_inDispatchMessageCount(0)
     , m_inDispatchMessageMarkedDispatchWhenWaitingForSyncReplyCount(0)
     , m_didReceiveInvalidMessage(false)
@@ -460,6 +461,8 @@
         m_pendingSyncReplies.append(PendingSyncReply(syncRequestID));
     }
 
+    ++m_inSendSyncCount;
+
     // First send the message.
     sendMessage(encoder, DispatchMessageEvenWhenWaitingForSyncReply);
 
@@ -468,6 +471,8 @@
     RefPtr<Connection> protect(this);
     OwnPtr<MessageDecoder> reply = waitForSyncReply(syncRequestID, timeout, syncSendFlags);
 
+    --m_inSendSyncCount;
+
     // Finally, pop the pending sync reply information.
     {
         MutexLocker locker(m_syncReplyStateMutex);

Modified: trunk/Source/WebKit2/Platform/CoreIPC/Connection.h (153487 => 153488)


--- trunk/Source/WebKit2/Platform/CoreIPC/Connection.h	2013-07-30 19:42:31 UTC (rev 153487)
+++ trunk/Source/WebKit2/Platform/CoreIPC/Connection.h	2013-07-30 19:45:09 UTC (rev 153488)
@@ -184,6 +184,8 @@
     void incrementDispatchMessageMarkedDispatchWhenWaitingForSyncReplyCount() { ++m_inDispatchMessageMarkedDispatchWhenWaitingForSyncReplyCount; }
     void decrementDispatchMessageMarkedDispatchWhenWaitingForSyncReplyCount() { --m_inDispatchMessageMarkedDispatchWhenWaitingForSyncReplyCount; }
 
+    bool inSendSync() const { return m_inSendSyncCount; }
+
 private:
     Connection(Identifier, bool isServer, Client*, WebCore::RunLoop* clientRunLoop);
     void platformInitialize(Identifier);
@@ -235,6 +237,7 @@
 
     HashMap<StringReference, std::pair<RefPtr<WorkQueue>, RefPtr<WorkQueueMessageReceiver>>> m_workQueueMessageReceivers;
 
+    unsigned m_inSendSyncCount;
     unsigned m_inDispatchMessageCount;
     unsigned m_inDispatchMessageMarkedDispatchWhenWaitingForSyncReplyCount;
     bool m_didReceiveInvalidMessage;

Modified: trunk/Source/WebKit2/PluginProcess/PluginControllerProxy.cpp (153487 => 153488)


--- trunk/Source/WebKit2/PluginProcess/PluginControllerProxy.cpp	2013-07-30 19:42:31 UTC (rev 153487)
+++ trunk/Source/WebKit2/PluginProcess/PluginControllerProxy.cpp	2013-07-30 19:45:09 UTC (rev 153488)
@@ -143,7 +143,8 @@
 {
     ASSERT(m_plugin);
 
-    if (m_pluginDestructionProtectCount) {
+    // FIXME: Consider removing m_pluginDestructionProtectCount and always use inSendSync here.
+    if (m_pluginDestructionProtectCount || m_connection->connection()->inSendSync()) {
         // We have plug-in code on the stack so we can't destroy it right now.
         // Destroy it later.
         m_pluginDestroyTimer.startOneShot(0);

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to