Title: [165040] trunk/Source/_javascript_Core
- Revision
- 165040
- Author
- commit-qu...@webkit.org
- Date
- 2014-03-03 22:57:59 -0800 (Mon, 03 Mar 2014)
Log Message
Web Inspector: Avoid too early deref caused by RemoteInspectorXPCConnection::close
https://bugs.webkit.org/show_bug.cgi?id=129631
Patch by Joseph Pecoraro <pecor...@apple.com> on 2014-03-03
Reviewed by Timothy Hatcher.
Avoid deref() too early if a client calls close(). The xpc_connection_close
will cause another XPC_ERROR event to come in from the queue, deref then.
Likewise, protect multithreaded access to m_client. If a client calls
close() we want to immediately clear the pointer to prevent calls to it.
Overall the multi-threading aspects of RemoteInspectorXPCConnection are
growing too complicated for probably little benefit. We may want to
clean this up later.
* inspector/remote/RemoteInspector.mm:
(Inspector::RemoteInspector::xpcConnectionFailed):
* inspector/remote/RemoteInspectorXPCConnection.h:
* inspector/remote/RemoteInspectorXPCConnection.mm:
(Inspector::RemoteInspectorXPCConnection::RemoteInspectorXPCConnection):
(Inspector::RemoteInspectorXPCConnection::close):
(Inspector::RemoteInspectorXPCConnection::closeOnQueue):
(Inspector::RemoteInspectorXPCConnection::deserializeMessage):
(Inspector::RemoteInspectorXPCConnection::handleEvent):
(Inspector::RemoteInspectorXPCConnection::sendMessage):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (165039 => 165040)
--- trunk/Source/_javascript_Core/ChangeLog 2014-03-04 06:52:56 UTC (rev 165039)
+++ trunk/Source/_javascript_Core/ChangeLog 2014-03-04 06:57:59 UTC (rev 165040)
@@ -1,3 +1,30 @@
+2014-03-03 Joseph Pecoraro <pecor...@apple.com>
+
+ Web Inspector: Avoid too early deref caused by RemoteInspectorXPCConnection::close
+ https://bugs.webkit.org/show_bug.cgi?id=129631
+
+ Reviewed by Timothy Hatcher.
+
+ Avoid deref() too early if a client calls close(). The xpc_connection_close
+ will cause another XPC_ERROR event to come in from the queue, deref then.
+ Likewise, protect multithreaded access to m_client. If a client calls
+ close() we want to immediately clear the pointer to prevent calls to it.
+
+ Overall the multi-threading aspects of RemoteInspectorXPCConnection are
+ growing too complicated for probably little benefit. We may want to
+ clean this up later.
+
+ * inspector/remote/RemoteInspector.mm:
+ (Inspector::RemoteInspector::xpcConnectionFailed):
+ * inspector/remote/RemoteInspectorXPCConnection.h:
+ * inspector/remote/RemoteInspectorXPCConnection.mm:
+ (Inspector::RemoteInspectorXPCConnection::RemoteInspectorXPCConnection):
+ (Inspector::RemoteInspectorXPCConnection::close):
+ (Inspector::RemoteInspectorXPCConnection::closeOnQueue):
+ (Inspector::RemoteInspectorXPCConnection::deserializeMessage):
+ (Inspector::RemoteInspectorXPCConnection::handleEvent):
+ (Inspector::RemoteInspectorXPCConnection::sendMessage):
+
2014-03-03 Michael Saboff <msab...@apple.com>
AbstractMacroAssembler::CachedTempRegister should start out invalid
Modified: trunk/Source/_javascript_Core/inspector/remote/RemoteInspector.mm (165039 => 165040)
--- trunk/Source/_javascript_Core/inspector/remote/RemoteInspector.mm 2014-03-04 06:52:56 UTC (rev 165039)
+++ trunk/Source/_javascript_Core/inspector/remote/RemoteInspector.mm 2014-03-04 06:57:59 UTC (rev 165040)
@@ -265,6 +265,8 @@
void RemoteInspector::xpcConnectionFailed(RemoteInspectorXPCConnection* connection)
{
std::lock_guard<std::mutex> lock(m_mutex);
+
+ ASSERT(connection == m_xpcConnection);
if (connection != m_xpcConnection)
return;
@@ -276,10 +278,8 @@
updateHasActiveDebugSession();
- if (m_xpcConnection) {
- m_xpcConnection->close();
- m_xpcConnection = nullptr;
- }
+ // The connection will close itself.
+ m_xpcConnection = nullptr;
}
void RemoteInspector::xpcConnectionUnhandledMessage(RemoteInspectorXPCConnection*, xpc_object_t)
Modified: trunk/Source/_javascript_Core/inspector/remote/RemoteInspectorXPCConnection.h (165039 => 165040)
--- trunk/Source/_javascript_Core/inspector/remote/RemoteInspectorXPCConnection.h 2014-03-04 06:52:56 UTC (rev 165039)
+++ trunk/Source/_javascript_Core/inspector/remote/RemoteInspectorXPCConnection.h 2014-03-04 06:57:59 UTC (rev 165040)
@@ -29,6 +29,7 @@
#define RemoteInspectorXPCConnection_h
#import <dispatch/dispatch.h>
+#import <mutex>
#import <wtf/ThreadSafeRefCounted.h>
#import <xpc/xpc.h>
@@ -56,7 +57,12 @@
private:
NSDictionary *deserializeMessage(xpc_object_t);
void handleEvent(xpc_object_t);
+ void closeOnQueue();
+ // We handle XPC events on the queue, but a client may call close() on any queue.
+ // We make sure that m_client is thread safe and immediately cleared in close().
+ std::mutex m_mutex;
+
xpc_connection_t m_connection;
dispatch_queue_t m_queue;
Client* m_client;
Modified: trunk/Source/_javascript_Core/inspector/remote/RemoteInspectorXPCConnection.mm (165039 => 165040)
--- trunk/Source/_javascript_Core/inspector/remote/RemoteInspectorXPCConnection.mm 2014-03-04 06:52:56 UTC (rev 165039)
+++ trunk/Source/_javascript_Core/inspector/remote/RemoteInspectorXPCConnection.mm 2014-03-04 06:57:59 UTC (rev 165040)
@@ -30,6 +30,7 @@
#import <Foundation/Foundation.h>
#import <wtf/Assertions.h>
+#import <wtf/Ref.h>
#if __has_include(<CoreFoundation/CFXPCBridge.h>)
#import <CoreFoundation/CFXPCBridge.h>
@@ -59,7 +60,7 @@
handleEvent(object);
});
- // Balanced by deref in close.
+ // Balanced by deref when the xpc_connection receives XPC_ERROR.
ref();
xpc_connection_resume(m_connection);
@@ -74,24 +75,30 @@
void RemoteInspectorXPCConnection::close()
{
- if (m_closed)
- return;
+ std::lock_guard<std::mutex> lock(m_mutex);
m_closed = true;
+ m_client = nullptr;
dispatch_async(m_queue, ^{
+ std::lock_guard<std::mutex> lock(m_mutex);
+ // This will trigger one last XPC_ERROR_CONNECTION_INVALID event on the queue and deref us.
+ closeOnQueue();
+ });
+}
+
+void RemoteInspectorXPCConnection::closeOnQueue()
+{
+ if (m_connection) {
xpc_connection_cancel(m_connection);
xpc_release(m_connection);
m_connection = NULL;
+ }
+ if (m_queue) {
dispatch_release(m_queue);
m_queue = NULL;
-
- m_client = nullptr;
-
- // Balance the ref in our constructor.
- deref();
- });
+ }
}
NSDictionary *RemoteInspectorXPCConnection::deserializeMessage(xpc_object_t object)
@@ -101,6 +108,7 @@
xpc_object_t xpcDictionary = xpc_dictionary_get_value(object, RemoteInspectorXPCConnectionSerializedMessageKey);
if (!xpcDictionary || xpc_get_type(xpcDictionary) != XPC_TYPE_DICTIONARY) {
+ std::lock_guard<std::mutex> lock(m_mutex);
if (m_client)
m_client->xpcConnectionUnhandledMessage(this, object);
return nil;
@@ -114,8 +122,17 @@
void RemoteInspectorXPCConnection::handleEvent(xpc_object_t object)
{
if (xpc_get_type(object) == XPC_TYPE_ERROR) {
+ std::lock_guard<std::mutex> lock(m_mutex);
if (m_client)
m_client->xpcConnectionFailed(this);
+
+ m_closed = true;
+ m_client = nullptr;
+ closeOnQueue();
+
+ // This is the last event we will ever receive from the connection.
+ // Balance the ref() in the constructor.
+ deref();
return;
}
@@ -125,12 +142,14 @@
NSString *message = [dataDictionary objectForKey:RemoteInspectorXPCConnectionMessageNameKey];
NSDictionary *userInfo = [dataDictionary objectForKey:RemoteInspectorXPCConnectionUserInfoKey];
+ std::lock_guard<std::mutex> lock(m_mutex);
if (m_client)
m_client->xpcConnectionReceivedMessage(this, message, userInfo);
}
void RemoteInspectorXPCConnection::sendMessage(NSString *messageName, NSDictionary *userInfo)
{
+ ASSERT(!m_closed);
if (m_closed)
return;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
