Title: [264806] trunk/Source/WebKit
- Revision
- 264806
- Author
- [email protected]
- Date
- 2020-07-23 16:54:23 -0700 (Thu, 23 Jul 2020)
Log Message
Crash in block in WebKit::LaunchServicesDatabaseObserver::LaunchServicesDatabaseObserver
https://bugs.webkit.org/show_bug.cgi?id=214702
Reviewed by Chris Dumez.
Calling xpc_connection_send_message in the block in WebKit::LaunchServicesDatabaseObserver::LaunchServicesDatabaseObserver will
sometimes crash because the XPC connection object is invalid. The block is called on a secondary thread, and the m_connections
member variable needs to be protected by a lock. Also, make sure the XPC connections objects in m_connections are valid by
retaining them when captured in a block. This is done by redeclaring a local XPC connection variable as an OSObjectPtr instead
of a raw XPC connection object.
No new tests, since I have not been able to reproduce this issue.
* NetworkProcess/cocoa/LaunchServicesDatabaseObserver.h:
* NetworkProcess/cocoa/LaunchServicesDatabaseObserver.mm:
(WebKit::LaunchServicesDatabaseObserver::LaunchServicesDatabaseObserver):
(WebKit::LaunchServicesDatabaseObserver::startObserving):
(WebKit::LaunchServicesDatabaseObserver::handleEvent):
* Shared/Cocoa/XPCEndpoint.mm:
(WebKit::XPCEndpoint::XPCEndpoint):
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (264805 => 264806)
--- trunk/Source/WebKit/ChangeLog 2020-07-23 23:42:42 UTC (rev 264805)
+++ trunk/Source/WebKit/ChangeLog 2020-07-23 23:54:23 UTC (rev 264806)
@@ -1,3 +1,26 @@
+2020-07-23 Per Arne Vollan <[email protected]>
+
+ Crash in block in WebKit::LaunchServicesDatabaseObserver::LaunchServicesDatabaseObserver
+ https://bugs.webkit.org/show_bug.cgi?id=214702
+
+ Reviewed by Chris Dumez.
+
+ Calling xpc_connection_send_message in the block in WebKit::LaunchServicesDatabaseObserver::LaunchServicesDatabaseObserver will
+ sometimes crash because the XPC connection object is invalid. The block is called on a secondary thread, and the m_connections
+ member variable needs to be protected by a lock. Also, make sure the XPC connections objects in m_connections are valid by
+ retaining them when captured in a block. This is done by redeclaring a local XPC connection variable as an OSObjectPtr instead
+ of a raw XPC connection object.
+
+ No new tests, since I have not been able to reproduce this issue.
+
+ * NetworkProcess/cocoa/LaunchServicesDatabaseObserver.h:
+ * NetworkProcess/cocoa/LaunchServicesDatabaseObserver.mm:
+ (WebKit::LaunchServicesDatabaseObserver::LaunchServicesDatabaseObserver):
+ (WebKit::LaunchServicesDatabaseObserver::startObserving):
+ (WebKit::LaunchServicesDatabaseObserver::handleEvent):
+ * Shared/Cocoa/XPCEndpoint.mm:
+ (WebKit::XPCEndpoint::XPCEndpoint):
+
2020-07-23 Brian Burg <[email protected]>
Web Inspector: developerExtrasEnabled should be respected when opening local Web Inspector (part 2)
Modified: trunk/Source/WebKit/NetworkProcess/cocoa/LaunchServicesDatabaseObserver.h (264805 => 264806)
--- trunk/Source/WebKit/NetworkProcess/cocoa/LaunchServicesDatabaseObserver.h 2020-07-23 23:42:42 UTC (rev 264805)
+++ trunk/Source/WebKit/NetworkProcess/cocoa/LaunchServicesDatabaseObserver.h 2020-07-23 23:54:23 UTC (rev 264806)
@@ -28,6 +28,7 @@
#include "NetworkProcess.h"
#include "NetworkProcessSupplement.h"
#include "XPCEndpoint.h"
+#include <wtf/Lock.h>
#include <wtf/OSObjectPtr.h>
#include <wtf/RetainPtr.h>
@@ -55,6 +56,7 @@
RetainPtr<id> m_observer;
Vector<OSObjectPtr<xpc_connection_t>> m_connections;
+ Lock m_connectionsLock;
};
}
Modified: trunk/Source/WebKit/NetworkProcess/cocoa/LaunchServicesDatabaseObserver.mm (264805 => 264806)
--- trunk/Source/WebKit/NetworkProcess/cocoa/LaunchServicesDatabaseObserver.mm 2020-07-23 23:42:42 UTC (rev 264805)
+++ trunk/Source/WebKit/NetworkProcess/cocoa/LaunchServicesDatabaseObserver.mm 2020-07-23 23:54:23 UTC (rev 264806)
@@ -56,8 +56,11 @@
xpc_dictionary_set_string(message.get(), XPCEndpoint::xpcMessageNameKey, LaunchServicesDatabaseXPCConstants::xpcUpdateLaunchServicesDatabaseMessageName);
xpc_dictionary_set_value(message.get(), LaunchServicesDatabaseXPCConstants::xpcLaunchServicesDatabaseKey, change);
- for (auto& connection : m_connections)
+ LockHolder holder(m_connectionsLock);
+ for (auto& connection : m_connections) {
+ RELEASE_ASSERT(xpc_get_type(connection.get()) == XPC_TYPE_CONNECTION);
xpc_connection_send_message(connection.get(), message.get());
+ }
}];
#endif
}
@@ -69,7 +72,10 @@
void LaunchServicesDatabaseObserver::startObserving(OSObjectPtr<xpc_connection_t> connection)
{
- m_connections.append(connection);
+ {
+ LockHolder holder(m_connectionsLock);
+ m_connections.append(connection);
+ }
#if HAVE(LSDATABASECONTEXT)
if (![databaseContext() respondsToSelector:@selector(addDatabaseChangeObserver4WebKit:)]) {
@@ -126,6 +132,7 @@
if (event != XPC_ERROR_CONNECTION_INVALID && event != XPC_ERROR_TERMINATION_IMMINENT)
return;
+ LockHolder holder(m_connectionsLock);
for (size_t i = 0; i < m_connections.size(); i++) {
if (m_connections[i].get() == connection) {
m_connections.remove(i);
Modified: trunk/Source/WebKit/Shared/Cocoa/XPCEndpoint.mm (264805 => 264806)
--- trunk/Source/WebKit/Shared/Cocoa/XPCEndpoint.mm 2020-07-23 23:42:42 UTC (rev 264805)
+++ trunk/Source/WebKit/Shared/Cocoa/XPCEndpoint.mm 2020-07-23 23:54:23 UTC (rev 264806)
@@ -40,9 +40,9 @@
xpc_type_t type = xpc_get_type(message);
if (type == XPC_TYPE_CONNECTION) {
- xpc_connection_t connection = message;
+ OSObjectPtr<xpc_connection_t> connection = message;
audit_token_t auditToken;
- xpc_connection_get_audit_token(connection, &auditToken);
+ xpc_connection_get_audit_token(connection.get(), &auditToken);
if (!WTF::hasEntitlement(auditToken, "com.apple.private.webkit.use-xpc-endpoint")) {
// Uncomment before landing; this is commented out because the bots does not seem to update the entitlements on incremental builds.
@@ -49,11 +49,11 @@
// WTFLogAlways("Audit token does not have required entitlement");
// return;
}
- xpc_connection_set_target_queue(connection, dispatch_get_main_queue());
- xpc_connection_set_event_handler(connection, ^(xpc_object_t event) {
- handleEvent(connection, event);
+ xpc_connection_set_target_queue(connection.get(), dispatch_get_main_queue());
+ xpc_connection_set_event_handler(connection.get(), ^(xpc_object_t event) {
+ handleEvent(connection.get(), event);
});
- xpc_connection_resume(connection);
+ xpc_connection_resume(connection.get());
}
});
xpc_connection_resume(m_connection.get());
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
