Changeset 294361 in webkit

Timestamp:

May 17, 2022, 4:23:29 PM (4 years ago)

Author:

Patrick Angle

Message:

[Cocoa] Web Driver: RemoteAutomationTargets marked for termination and targets that have never been paired may briefly be indistinguishable in application target listing
​https://bugs.webkit.org/show_bug.cgi?id=240524
<rdar://93430106>

Reviewed by BJ Burg and Devin Rousso.

A RemoteAutomationTarget has multiple possible entries it may provide in an application listing to webinspectord,
which are then interpreted by webinspectord to determine the current state of that target. The key pieces for this bug
are that a WIRConnectionIdentifierKey which when present is taken to mean that the target is no longer pristine and
WIRAutomationTargetIsPairedKey which when true indicates that the target is paired. When a connection identifier is
present and we are no longer paired, the connection is considered terminated.

There exists a race condition where webinspectord preemptively set the targets state to "Terminated" upon sending a
close request in order to make sure that the target is no longer used. Upon receiving the close message for the target
the connection identifier is removed from the map of known connection identifiers before scheduling the actual target to
be torn down asyncronously. In most cases this works fine, but sometimes an application listing is created and sent
between removing the identifier from the map of known connections and actually tearing down the target, and that is
where this issue occurs. We will in that case send a listing for the automation target with no connection identifier
(since we removed it from the map already), which is then interpreted as a listing for a pristine automation target. At
that point, it is possible that automation sessions that are being rapidly created and destroyed will attempt to pair
with the seemingly pristine existing session, even though that session is about to be torn down.

• Source/JavaScriptCore/inspector/remote/RemoteAutomationTarget.h:
• Source/JavaScriptCore/inspector/remote/cocoa/RemoteConnectionToTargetCocoa.mm:

(Inspector::RemoteConnectionToTarget::close):

• Source/JavaScriptCore/inspector/remote/cocoa/RemoteInspectorCocoa.mm:

(Inspector::RemoteInspector::listingForAutomationTarget const):

Canonical link: ​https://commits.webkit.org/250664@main

Location:

trunk/Source/JavaScriptCore/inspector/remote

Files:

• RemoteAutomationTarget.h (modified) (2 diffs)
• cocoa/RemoteConnectionToTargetCocoa.mm (modified) (1 diff)
• cocoa/RemoteInspectorCocoa.mm (modified) (1 diff)

trunk/Source/JavaScriptCore/inspector/remote/RemoteAutomationTarget.h

@@ -42 +42 @@
     void setIsPaired(bool);
 
+    bool isPendingTermination() const { return m_pendingTermination; }
+    void setIsPendingTermination() { m_pendingTermination = true; }
+
     virtual String name() const = 0;
     RemoteControllableTarget::Type type() const override { return RemoteControllableTarget::Type::Automation; }
@@ -48 +51 @@
 private:
     bool m_paired { false };
+    bool m_pendingTermination { false };
 };
 

trunk/Source/JavaScriptCore/inspector/remote/cocoa/RemoteConnectionToTargetCocoa.mm

@@ -199 +199 @@
 {
     auto targetIdentifier = m_target ? m_target->targetIdentifier() : 0;
+
+    if (auto* automationTarget = dynamicDowncast<RemoteAutomationTarget>(m_target))
+        automationTarget->setIsPendingTermination();
     
     dispatchAsyncOnTarget([this, targetIdentifier, strongThis = Ref { *this }]() {

trunk/Source/JavaScriptCore/inspector/remote/cocoa/RemoteInspectorCocoa.mm

@@ -467 +467 @@
     ASSERT(isMainThread());
 
+    if (target.isPendingTermination())
+        return nullptr;
+
     RetainPtr<NSMutableDictionary> listing = adoptNS([[NSMutableDictionary alloc] init]);
     [listing setObject:@(target.targetIdentifier()) forKey:WIRTargetIdentifierKey];