Title: [278413] trunk/Source/_javascript_Core
- Revision
- 278413
- Author
- [email protected]
- Date
- 2021-06-03 12:32:22 -0700 (Thu, 03 Jun 2021)
Log Message
Web Inspector: [Cocoa] `RemoteInspector` won't connect to a new relay if it hasn't yet failed to communicate with a previously connected relay
https://bugs.webkit.org/show_bug.cgi?id=226539
Reviewed by Devin Rousso.
`RemoteInspector` communicates with a relay daemon running on the same device in order to send updates like new
or removed inspectable targets and receive changes to settings like automatic debugging. The relay daemon then
communicates with a client that connects for debugging. Only one relay daemon should ever be running at a time,
and its lifecycle is managed separately from _javascript_Core.
RemoteInspector holds a RefPtr to its connection to this relay, and only clears this pointer upon a failure to
communicate over the XPC connection or a known disconnection. However, it is possible, and in some cases likely
(for example the relay restarting from a brief client disconnection and reconnection), that we can be informed
of a newly launched relay being available while still thinking we are connected to the old relay, as we have not
yet sent a message and triggered a failure in the interim period of time.
To correct this we now send a simple message any time `setupXPCConnectionIfNeeded` is called if we have an
existing RefPtr to a relay connection in order to verify the connection is still functional. We now also retry
to connect to a relay upon failure in order to create a new connection to the current relay.
In order to prevent entering a retry loop where every subsequent retry's failure results in another retry
forever, a flag to retry connecting is set when a call to setupXPCConnectionIfNeeded is made while we already
have a RefPtr to a relay connection. On failure if we are in this special state we will retry once to connect
but subsequent failures will not automatically reattempt a connection.
* inspector/remote/RemoteInspector.h:
* inspector/remote/cocoa/RemoteInspectorCocoa.mm:
(Inspector::RemoteInspector::stopInternal):
- Clear the retry connection flag when stopping in an orderly fashion.
(Inspector::RemoteInspector::setupXPCConnectionIfNeeded):
- Set the retry connection flag and send a simple message if we already have a relay connection in order to make
sure the connection is either still valid or is torn down properly on failure.
(Inspector::RemoteInspector::xpcConnectionFailed):
- If the retry flag is set, schedule a retry and clear the retry flag.
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (278412 => 278413)
--- trunk/Source/_javascript_Core/ChangeLog 2021-06-03 18:04:30 UTC (rev 278412)
+++ trunk/Source/_javascript_Core/ChangeLog 2021-06-03 19:32:22 UTC (rev 278413)
@@ -1,3 +1,40 @@
+2021-06-03 Patrick Angle <[email protected]>
+
+ Web Inspector: [Cocoa] `RemoteInspector` won't connect to a new relay if it hasn't yet failed to communicate with a previously connected relay
+ https://bugs.webkit.org/show_bug.cgi?id=226539
+
+ Reviewed by Devin Rousso.
+
+ `RemoteInspector` communicates with a relay daemon running on the same device in order to send updates like new
+ or removed inspectable targets and receive changes to settings like automatic debugging. The relay daemon then
+ communicates with a client that connects for debugging. Only one relay daemon should ever be running at a time,
+ and its lifecycle is managed separately from _javascript_Core.
+
+ RemoteInspector holds a RefPtr to its connection to this relay, and only clears this pointer upon a failure to
+ communicate over the XPC connection or a known disconnection. However, it is possible, and in some cases likely
+ (for example the relay restarting from a brief client disconnection and reconnection), that we can be informed
+ of a newly launched relay being available while still thinking we are connected to the old relay, as we have not
+ yet sent a message and triggered a failure in the interim period of time.
+
+ To correct this we now send a simple message any time `setupXPCConnectionIfNeeded` is called if we have an
+ existing RefPtr to a relay connection in order to verify the connection is still functional. We now also retry
+ to connect to a relay upon failure in order to create a new connection to the current relay.
+
+ In order to prevent entering a retry loop where every subsequent retry's failure results in another retry
+ forever, a flag to retry connecting is set when a call to setupXPCConnectionIfNeeded is made while we already
+ have a RefPtr to a relay connection. On failure if we are in this special state we will retry once to connect
+ but subsequent failures will not automatically reattempt a connection.
+
+ * inspector/remote/RemoteInspector.h:
+ * inspector/remote/cocoa/RemoteInspectorCocoa.mm:
+ (Inspector::RemoteInspector::stopInternal):
+ - Clear the retry connection flag when stopping in an orderly fashion.
+ (Inspector::RemoteInspector::setupXPCConnectionIfNeeded):
+ - Set the retry connection flag and send a simple message if we already have a relay connection in order to make
+ sure the connection is either still valid or is torn down properly on failure.
+ (Inspector::RemoteInspector::xpcConnectionFailed):
+ - If the retry flag is set, schedule a retry and clear the retry flag.
+
2021-06-02 Robin Morisset <[email protected]>
B3MoveConstants should filter directly on Values, and only create ValueKeys when useful
Modified: trunk/Source/_javascript_Core/inspector/remote/RemoteInspector.h (278412 => 278413)
--- trunk/Source/_javascript_Core/inspector/remote/RemoteInspector.h 2021-06-03 18:04:30 UTC (rev 278412)
+++ trunk/Source/_javascript_Core/inspector/remote/RemoteInspector.h 2021-06-03 19:32:22 UTC (rev 278413)
@@ -260,6 +260,7 @@
#if PLATFORM(COCOA)
RefPtr<RemoteInspectorXPCConnection> m_relayConnection;
+ bool m_shouldReconnectToRelayOnFailure { false };
#endif
#if USE(GLIB)
RefPtr<SocketConnection> m_socketConnection;
Modified: trunk/Source/_javascript_Core/inspector/remote/cocoa/RemoteInspectorCocoa.mm (278412 => 278413)
--- trunk/Source/_javascript_Core/inspector/remote/cocoa/RemoteInspectorCocoa.mm 2021-06-03 18:04:30 UTC (rev 278412)
+++ trunk/Source/_javascript_Core/inspector/remote/cocoa/RemoteInspectorCocoa.mm 2021-06-03 19:32:22 UTC (rev 278413)
@@ -265,6 +265,8 @@
m_relayConnection = nullptr;
}
+ m_shouldReconnectToRelayOnFailure = false;
+
notify_cancel(m_notifyToken);
}
@@ -272,12 +274,19 @@
{
Locker locker { m_mutex };
- if (m_relayConnection)
+ if (m_relayConnection) {
+ m_shouldReconnectToRelayOnFailure = true;
+
+ // Send a simple message to make sure the connection is still open.
+ m_relayConnection->sendMessage(@"check", nil);
return;
+ }
auto connection = adoptOSObject(xpc_connection_create_mach_service(WIRXPCMachPortName, m_xpcQueue, 0));
- if (!connection)
+ if (!connection) {
+ WTFLogAlways("RemoteInspector failed to create XPC connection.");
return;
+ }
m_relayConnection = adoptRef(new RemoteInspectorXPCConnection(connection.get(), m_xpcQueue, this));
m_relayConnection->sendMessage(@"syn", nil); // Send a simple message to initialize the XPC connection.
@@ -363,6 +372,19 @@
// The XPC connection will close itself.
m_relayConnection = nullptr;
+
+ if (!m_shouldReconnectToRelayOnFailure) {
+ WTFLogAlways("RemoteInspector XPC connection to relay failed.");
+ return;
+ }
+
+ m_shouldReconnectToRelayOnFailure = false;
+ WTFLogAlways("RemoteInspector XPC connection to relay failed, reconnecting in 1 second...");
+
+ // Schedule setting up a new connection, since we currently are holding a lock needed to create a new connection.
+ dispatch_after(dispatch_time(DISPATCH_TIME_NOW, 1 * NSEC_PER_SEC), dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
+ RemoteInspector::singleton().setupXPCConnectionIfNeeded();
+ });
}
void RemoteInspector::xpcConnectionUnhandledMessage(RemoteInspectorXPCConnection*, xpc_object_t)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
