Unreviewed, rolling out r175806.
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 12 Nov 2014 21:48:51 +0000 (21:48 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 12 Nov 2014 21:48:51 +0000 (21:48 +0000)
https://bugs.webkit.org/show_bug.cgi?id=138666

Capturing CString is not thread safe (Requested by anttik on
#webkit).

Reverted changeset:

"[WK2] Use C++ lambdas in IPC::Connection"
https://bugs.webkit.org/show_bug.cgi?id=138018
http://trac.webkit.org/changeset/175806

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@176036 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit2/ChangeLog
Source/WebKit2/Platform/IPC/Connection.cpp
Source/WebKit2/Platform/IPC/mac/ConnectionMac.mm
Source/WebKit2/Platform/IPC/unix/ConnectionUnix.cpp

index 765d826..00ed16f 100644 (file)
@@ -1,3 +1,17 @@
+2014-11-12  Commit Queue  <commit-queue@webkit.org>
+
+        Unreviewed, rolling out r175806.
+        https://bugs.webkit.org/show_bug.cgi?id=138666
+
+        Capturing CString is not thread safe (Requested by anttik on
+        #webkit).
+
+        Reverted changeset:
+
+        "[WK2] Use C++ lambdas in IPC::Connection"
+        https://bugs.webkit.org/show_bug.cgi?id=138018
+        http://trac.webkit.org/changeset/175806
+
 2014-11-12  Tim Horton  <timothy_horton@apple.com>
 
         Make action menus much more reliable
index 242601b..0cc5483 100644 (file)
@@ -164,13 +164,8 @@ bool Connection::SyncMessageState::processIncomingMessage(Connection* connection
     {
         MutexLocker locker(m_mutex);
         
-        if (m_didScheduleDispatchMessagesWorkSet.add(connection).isNewEntry) {
-            RefPtr<SyncMessageState> protectedThis(this);
-            RefPtr<Connection> protectedConnection(connection);
-            m_runLoop.dispatch([protectedThis, protectedConnection] {
-                protectedThis->dispatchMessageAndResetDidScheduleDispatchMessagesForConnection(protectedConnection.get());
-            });
-        }
+        if (m_didScheduleDispatchMessagesWorkSet.add(connection).isNewEntry)
+            m_runLoop.dispatch(bind(&SyncMessageState::dispatchMessageAndResetDidScheduleDispatchMessagesForConnection, this, RefPtr<Connection>(connection)));
 
         m_messagesToDispatchWhileWaitingForSyncReply.append(WTF::move(connectionAndIncomingMessage));
     }
@@ -301,8 +296,10 @@ void Connection::removeWorkQueueMessageReceiver(StringReference messageReceiverN
     });
 }
 
-void Connection::dispatchWorkQueueMessageReceiverMessage(WorkQueueMessageReceiver* workQueueMessageReceiver, MessageDecoder* decoder)
+void Connection::dispatchWorkQueueMessageReceiverMessage(WorkQueueMessageReceiver* workQueueMessageReceiver, MessageDecoder* incomingMessageDecoder)
 {
+    std::unique_ptr<MessageDecoder> decoder(incomingMessageDecoder);
+
     if (!decoder->isSyncMessage()) {
         workQueueMessageReceiver->didReceiveMessage(this, *decoder);
         return;
@@ -343,12 +340,9 @@ void Connection::invalidate()
     }
     
     // Reset the client.
-    m_client = nullptr;
+    m_client = 0;
 
-    RefPtr<Connection> protectedThis(this);
-    m_connectionQueue->dispatch([protectedThis] {
-        protectedThis->platformInvalidate();
-    });
+    m_connectionQueue->dispatch(WTF::bind(&Connection::platformInvalidate, this));
 }
 
 void Connection::markCurrentlyDispatchedMessageAsInvalid()
@@ -387,10 +381,7 @@ bool Connection::sendMessage(std::unique_ptr<MessageEncoder> encoder, unsigned m
     }
     
     // FIXME: We should add a boolean flag so we don't call this when work has already been scheduled.
-    RefPtr<Connection> protectedThis(this);
-    m_connectionQueue->dispatch([protectedThis] {
-        protectedThis->sendOutgoingMessages();
-    });
+    m_connectionQueue->dispatch(WTF::bind(&Connection::sendOutgoingMessages, this));
     return true;
 }
 
@@ -638,27 +629,23 @@ void Connection::processIncomingMessage(std::unique_ptr<MessageDecoder> message)
     }
 
     if (!m_workQueueMessageReceivers.isValidKey(message->messageReceiverName())) {
-        // Something might have gone wrong when decoding the message. In that case, encode the message length
-        // so we can figure out if this happens for certain message lengths.
-        CString messageReceiverName = message->messageReceiverName().isEmpty() ? "<unknown message>" : message->messageReceiverName().toString();
-        CString messageName = message->messageName().isEmpty() ? String::format("<message length: %zu bytes>", message->length()).utf8() : message->messageReceiverName().toString();
-
-        RefPtr<Connection> protectedThis(this);
-        m_clientRunLoop.dispatch([protectedThis, messageReceiverName, messageName] {
-            protectedThis->dispatchDidReceiveInvalidMessage(messageReceiverName, messageName);
-        });
+        if (message->messageReceiverName().isEmpty() && message->messageName().isEmpty()) {
+            // Something went wrong when decoding the message. Encode the message length so we can figure out if this
+            // happens for certain message lengths.
+            CString messageReceiverName = "<unknown message>";
+            CString messageName = String::format("<message length: %zu bytes>", message->length()).utf8();
+
+            m_clientRunLoop.dispatch(bind(&Connection::dispatchDidReceiveInvalidMessage, this, messageReceiverName, messageName));
+            return;
+        }
+
+        m_clientRunLoop.dispatch(bind(&Connection::dispatchDidReceiveInvalidMessage, this, message->messageReceiverName().toString(), message->messageName().toString()));
         return;
     }
 
     auto it = m_workQueueMessageReceivers.find(message->messageReceiverName());
     if (it != m_workQueueMessageReceivers.end()) {
-        RefPtr<Connection> protectedThis(this);
-        RefPtr<WorkQueueMessageReceiver>& workQueueMessageReceiver = it->value.second;
-        MessageDecoder* decoderPtr = message.release();
-        it->value.first->dispatch([protectedThis, workQueueMessageReceiver, decoderPtr] {
-            std::unique_ptr<MessageDecoder> decoder(decoderPtr);
-            protectedThis->dispatchWorkQueueMessageReceiverMessage(workQueueMessageReceiver.get(), decoder.get());
-        });
+        it->value.first->dispatch(bind(&Connection::dispatchWorkQueueMessageReceiverMessage, this, it->value.second, message.release()));
         return;
     }
 
@@ -814,10 +801,7 @@ void Connection::enqueueIncomingMessage(std::unique_ptr<MessageDecoder> incoming
         m_incomingMessages.append(WTF::move(incomingMessage));
     }
 
-    RefPtr<Connection> protectedThis(this);
-    m_clientRunLoop.dispatch([protectedThis] {
-        protectedThis->dispatchOneMessage();
-    });
+    m_clientRunLoop.dispatch(WTF::bind(&Connection::dispatchOneMessage, this));
 }
 
 void Connection::dispatchMessage(MessageDecoder& decoder)
index 3b75e8e..c0845ec 100644 (file)
@@ -361,11 +361,7 @@ bool Connection::sendOutgoingMessage(std::unique_ptr<MessageEncoder> encoder)
 void Connection::initializeDeadNameSource()
 {
     m_deadNameSource = dispatch_source_create(DISPATCH_SOURCE_TYPE_MACH_SEND, m_sendPort, 0, m_connectionQueue->dispatchQueue());
-
-    RefPtr<Connection> protectedThis(this);
-    dispatch_source_set_event_handler(m_deadNameSource, ^{
-        protectedThis->connectionDidClose();
-    });
+    dispatch_source_set_event_handler(m_deadNameSource, bind(&Connection::connectionDidClose, this));
 
     mach_port_t sendPort = m_sendPort;
     dispatch_source_set_cancel_handler(m_deadNameSource, ^{
@@ -508,12 +504,7 @@ void Connection::receiveSourceEventHandler()
     if (decoder->messageReceiverName() == "IPC" && decoder->messageName() == "SetExceptionPort") {
         if (m_isServer) {
             // Server connections aren't supposed to have their exception ports overriden. Treat this as an invalid message.
-            RefPtr<Connection> protectedThis(this);
-            CString messageReceiverNameString = decoder->messageReceiverName().toString();
-            CString messageNameString = decoder->messageName().toString();
-            m_clientRunLoop.dispatch([protectedThis, messageReceiverNameString, messageNameString] {
-                protectedThis->dispatchDidReceiveInvalidMessage(messageReceiverNameString, messageNameString);
-            });
+            m_clientRunLoop.dispatch(bind(&Connection::dispatchDidReceiveInvalidMessage, this, decoder->messageReceiverName().toString(), decoder->messageName().toString()));
             return;
         }
         MachPort exceptionPort;
index 4dd95fd..16be014 100644 (file)
@@ -388,27 +388,27 @@ bool Connection::open()
         }
     }
 
-    RefPtr<Connection> protectedThis(this);
     m_isConnected = true;
 #if PLATFORM(GTK)
+    RefPtr<Connection> protector(this);
     m_connectionQueue->registerSocketEventHandler(m_socketDescriptor,
-        [protectedThis] {
-            protectedThis->readyReadHandler();
+        [=] {
+            protector->readyReadHandler();
         },
-        [protectedThis] {
-            protectedThis->connectionDidClose();
+        [=] {
+            protector->connectionDidClose();
         });
 #elif PLATFORM(EFL)
+    RefPtr<Connection> protector(this);
     m_connectionQueue->registerSocketEventHandler(m_socketDescriptor,
-        [protectedThis] {
-            protectedThis->readyReadHandler();
+        [protector] {
+            protector->readyReadHandler();
         });
 #endif
 
-    // Schedule a call to readyReadHandler. Data may have arrived before installation of the signal handler.
-    m_connectionQueue->dispatch([protectedThis] {
-        protectedThis->readyReadHandler();
-    });
+    // Schedule a call to readyReadHandler. Data may have arrived before installation of the signal
+    // handler.
+    m_connectionQueue->dispatch(WTF::bind(&Connection::readyReadHandler, this));
 
     return true;
 }