Title: [294361] trunk/Source/_javascript_Core/inspector/remote
- Revision
- 294361
- Author
- pan...@apple.com
- Date
- 2022-05-17 16:23:29 -0700 (Tue, 17 May 2022)
Log Message
[Cocoa] Web Driver: `RemoteAutomationTarget`s 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/_javascript_Core/inspector/remote/RemoteAutomationTarget.h:
* Source/_javascript_Core/inspector/remote/cocoa/RemoteConnectionToTargetCocoa.mm:
(Inspector::RemoteConnectionToTarget::close):
* Source/_javascript_Core/inspector/remote/cocoa/RemoteInspectorCocoa.mm:
(Inspector::RemoteInspector::listingForAutomationTarget const):
Canonical link: https://commits.webkit.org/250664@main
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/inspector/remote/RemoteAutomationTarget.h (294360 => 294361)
--- trunk/Source/_javascript_Core/inspector/remote/RemoteAutomationTarget.h 2022-05-17 23:19:06 UTC (rev 294360)
+++ trunk/Source/_javascript_Core/inspector/remote/RemoteAutomationTarget.h 2022-05-17 23:23:29 UTC (rev 294361)
@@ -41,6 +41,9 @@
bool isPaired() const { return m_paired; }
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; }
bool remoteControlAllowed() const override { return !m_paired; };
@@ -47,6 +50,7 @@
private:
bool m_paired { false };
+ bool m_pendingTermination { false };
};
} // namespace Inspector
Modified: trunk/Source/_javascript_Core/inspector/remote/cocoa/RemoteConnectionToTargetCocoa.mm (294360 => 294361)
--- trunk/Source/_javascript_Core/inspector/remote/cocoa/RemoteConnectionToTargetCocoa.mm 2022-05-17 23:19:06 UTC (rev 294360)
+++ trunk/Source/_javascript_Core/inspector/remote/cocoa/RemoteConnectionToTargetCocoa.mm 2022-05-17 23:23:29 UTC (rev 294361)
@@ -198,6 +198,9 @@
void RemoteConnectionToTarget::close()
{
auto targetIdentifier = m_target ? m_target->targetIdentifier() : 0;
+
+ if (auto* automationTarget = dynamicDowncast<RemoteAutomationTarget>(m_target))
+ automationTarget->setIsPendingTermination();
dispatchAsyncOnTarget([this, targetIdentifier, strongThis = Ref { *this }]() {
Locker locker { m_targetMutex };
Modified: trunk/Source/_javascript_Core/inspector/remote/cocoa/RemoteInspectorCocoa.mm (294360 => 294361)
--- trunk/Source/_javascript_Core/inspector/remote/cocoa/RemoteInspectorCocoa.mm 2022-05-17 23:19:06 UTC (rev 294360)
+++ trunk/Source/_javascript_Core/inspector/remote/cocoa/RemoteInspectorCocoa.mm 2022-05-17 23:23:29 UTC (rev 294361)
@@ -466,6 +466,9 @@
// implemented by non-threadsafe JSC / WebCore classes such as JSGlobalObject or WebCore::Page.
ASSERT(isMainThread());
+ if (target.isPendingTermination())
+ return nullptr;
+
RetainPtr<NSMutableDictionary> listing = adoptNS([[NSMutableDictionary alloc] init]);
[listing setObject:@(target.targetIdentifier()) forKey:WIRTargetIdentifierKey];
[listing setObject:target.name() forKey:WIRSessionIdentifierKey];
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
