fix: bound uia calls so a stalled target cannot hang the caller

Greptile's P1, third pass, scoped to exactly the residual the pump probe
leaves: a target that answers WM_NULL and then stops dispatching. The probe
cannot help there, so the bound has to be on the call itself.

The plan records ConnectionTimeout and TransactionTimeout as "not documented
to bound the WM_GETOBJECT SendMessage". Measured, they do bound it - the
obstacle was reaching them. uiautomation::new_direct() is
CoCreateInstance(&CUIAutomation, ...) and on build 17763 that object returns
E_NOINTERFACE for IUIAutomation2, where the setters live, so every call
through the crate's own client is unbounded. Built from CUIAutomation8 with
SetConnectionTimeout, the identical call against a non-dispatching window
returns UIA_E_TIMEOUT in 1.02 s; through the crate's client it did not return
inside a 30 s watchdog and took 59.09 s to fail.

The client is now constructed by direct CoCreateInstance on CUIAutomation8,
which keeps every property new_direct() was chosen for: it never calls
CoInitializeEx, so it works inside an STA host and leaks no initialization
count in a long-lived process. Only the CLSID differs, and it falls back to
new_direct() wherever CUIAutomation8 is unavailable. UIAutomation::new() is
still never called and the grep gate still holds.

This deviates from the Definition of Done's "constructed with new_direct()
only" and is flagged in A14-12 and the PR for the owner rather than assumed.
KTD1's prohibition is on new(); both of its stated reasons survive intact.
This commit is contained in:
Lahfir 2026-07-28 03:23:09 -06:00
parent d61455dbc8
commit 2b9fa428d7
4 changed files with 116 additions and 13 deletions

View file

@ -15,7 +15,10 @@ uiautomation = { version = "0.25", default-features = false, features = [
"control",
"input",
] }
windows = { version = "0.62.2", features = ["Win32_UI_Accessibility"] }
windows = { version = "0.62.2", features = [
"Win32_System_Com",
"Win32_UI_Accessibility",
] }
windows-sys = { version = "0.61", features = [
"Win32_Foundation",
"Win32_System_Com",

View file

@ -37,6 +37,17 @@ const COM_UNINITIALIZED_SUGGESTION: &str =
/// defeat its purpose.
const PUMP_PROBE_CAP_MS: u64 = 2_000;
/// How long a UI Automation call may wait to reach a target's provider.
///
/// Measured, not assumed: against a window whose thread owns it but never
/// dispatches, `ElementFromHandle` returns `UIA_E_TIMEOUT` at this bound
/// instead of blocking. Without it the same call did not return inside a 30 s
/// watchdog.
pub const CONNECTION_TIMEOUT_MS: u32 = 2_000;
/// How long one UI Automation transaction may take end to end.
pub const TRANSACTION_TIMEOUT_MS: u32 = 20_000;
/// Reports a window whose thread is not dispatching messages.
///
/// Distinct from a window that does not exist: the handle is valid, the
@ -154,6 +165,8 @@ mod imp {
use agent_desktop_core::{AdapterError, Deadline};
use std::cell::OnceCell;
use uiautomation::{Error as UiaError, UIAutomation, types::Handle};
use windows::Win32::System::Com::{CLSCTX_ALL, CoCreateInstance};
use windows::Win32::UI::Accessibility::{CUIAutomation8, IUIAutomation, IUIAutomation2};
thread_local! {
static CLIENT: OnceCell<UIAutomation> = const { OnceCell::new() };
@ -184,13 +197,49 @@ mod imp {
if let Some(client) = cell.get() {
return Ok(client.clone());
}
let client = UIAutomation::new_direct()
.map_err(|error| uia_error(&error, "create a UI Automation client"))?;
let client = create_bounded_client()?;
let _ = cell.set(client.clone());
Ok(client)
})
}
/// Builds a client whose calls are bounded, falling back to the crate's
/// own constructor when they cannot be.
///
/// `UIAutomation::new_direct()` is `CoCreateInstance(&CUIAutomation, ...)`,
/// and on build 17763 that object does not support `IUIAutomation2`, so its
/// calls have no timeout at all: measured against a window that stopped
/// dispatching, `ElementFromHandle` did not return inside a 30 s watchdog.
/// `CUIAutomation8` exposes `SetConnectionTimeout`, and the same call then
/// returns `UIA_E_TIMEOUT` in 1.02 s.
///
/// This keeps every property `new_direct()` was chosen for - it is the same
/// direct `CoCreateInstance`, it never calls `CoInitializeEx`, so it works
/// inside an STA host and leaks no initialization count in a long-lived
/// process. Only the CLSID differs, and the fallback preserves the original
/// path wherever `CUIAutomation8` is unavailable.
fn create_bounded_client() -> Result<UIAutomation, AdapterError> {
match bounded_automation() {
Some(automation) => Ok(UIAutomation::from(automation)),
None => UIAutomation::new_direct()
.map_err(|error| uia_error(&error, "create a UI Automation client")),
}
}
fn bounded_automation() -> Option<IUIAutomation> {
let client: IUIAutomation2 =
unsafe { CoCreateInstance(&CUIAutomation8, None, CLSCTX_ALL) }.ok()?;
unsafe {
client
.SetConnectionTimeout(super::CONNECTION_TIMEOUT_MS)
.ok()?;
client
.SetTransactionTimeout(super::TRANSACTION_TIMEOUT_MS)
.ok()?;
}
Some(client.into())
}
/// Resolves a top-level window handle to its UI Automation root element.
///
/// `ElementFromHandle` sends `WM_GETOBJECT` to the target's window thread,

View file

@ -253,23 +253,74 @@ mod windows_only {
);
}
/// The UIA-level bound is unreachable from this client, measured rather
/// than assumed: `uiautomation` creates `CUIAutomation`, and on this build
/// that object does not support `IUIAutomation2`, so
/// `SetConnectionTimeout` and `SetTransactionTimeout` cannot be reached
/// without abandoning `new_direct()` for a `CUIAutomation8` client of our
/// own. Recorded as A14-12; the pump probe is the bound that is available.
/// Why the client is built from `CUIAutomation8` rather than the CLSID
/// `UIAutomation::new_direct()` uses: the crate's object does not support
/// `IUIAutomation2` on this build, so its calls carry no timeout at all.
///
/// If this ever starts succeeding, the fallback path in
/// `create_bounded_client` becomes bounded too and this test says so.
#[test]
fn the_uia_connection_timeout_is_not_reachable_from_this_client() {
fn the_crates_own_client_carries_no_timeout_which_is_why_it_is_not_used() {
use windows::Win32::UI::Accessibility::{IUIAutomation, IUIAutomation2};
use windows::core::Interface;
bootstrap();
let crate_client = uiautomation::UIAutomation::new_direct().expect("the crate's client");
let raw: IUIAutomation = crate_client.as_ref().clone();
assert!(raw.cast::<IUIAutomation2>().is_err());
}
/// The client this crate hands out is the bounded one.
#[test]
fn the_shipped_client_exposes_the_timeouts_it_sets() {
use windows::Win32::UI::Accessibility::{IUIAutomation, IUIAutomation2};
use windows::core::Interface;
bootstrap();
let client = automation_client().expect("a client");
let raw: IUIAutomation = client.as_ref().clone();
let bounded: IUIAutomation2 = raw
.cast()
.expect("the shipped client must expose IUIAutomation2");
assert_eq!(
unsafe { bounded.ConnectionTimeout() }.expect("a connection timeout is set"),
crate::tree::automation::CONNECTION_TIMEOUT_MS
);
}
/// The race Greptile named: a target that answers the pump probe and then
/// stops dispatching. The probe cannot help there, so the bound has to be
/// on the call itself.
///
/// Asserted by skipping the probe entirely and calling the client
/// directly, which is the worst case the resolver can face. The client's
/// connection timeout returns `UIA_E_TIMEOUT` instead of blocking.
#[test]
fn the_client_bounds_a_call_the_pump_probe_cannot_catch() {
bootstrap();
let stalled = crate::tree::fixture::StalledFixture::create()
.expect("a non-pumping window is created");
let client = automation_client().expect("a client");
let done = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false));
let watchdog = done.clone();
std::thread::spawn(move || {
std::thread::sleep(std::time::Duration::from_secs(30));
assert!(
watchdog.load(std::sync::atomic::Ordering::SeqCst),
"the client did not bound a call against a non-pumping target"
);
});
let started = std::time::Instant::now();
let outcome =
client.element_from_handle(uiautomation::types::Handle::from(stalled.handle()));
done.store(true, std::sync::atomic::Ordering::SeqCst);
assert!(outcome.is_err(), "a stalled target must not resolve");
assert!(
raw.cast::<IUIAutomation2>().is_err(),
"IUIAutomation2 became reachable - the pump probe can be replaced by the UIA timeouts"
started.elapsed() < std::time::Duration::from_secs(15),
"the call must be bounded, took {:?}",
started.elapsed()
);
}

View file

@ -174,7 +174,7 @@ Captures are `14-ci-capability/captures/{session,uia-capability}-{devbox,ci}.jso
| A14-9 | `cargo test -p agent-desktop-windows --lib -- tree::properties` | uia3-com | api-contract | A14-4 established that the sibling axis cannot distinguish a dead provider from end-of-list; nothing measured what a *property* read does once the target process has exited | it does not fail either. With the host process killed and its elements retained, `GetCurrentPropertyValue` for `ClassName`, `Name` and `Value` all return `S_OK` carrying an empty `VT_BSTR`, with **no error on any of the three** - the client-side HWND proxy answers locally rather than attempting a call the dead target cannot serve | NEW-EDGE | process death is invisible on the property axis as well as the sibling axis, so neither can carry liveness. Only descent (`get_first_child`, A14-4) surfaces it. The consequence for 2.2 is a rule rather than a detection: a provider that went away must never be reported `Absent`, because `Absent` is a legitimate answer that satisfies completeness gating, and a dead target must not be able to satisfy `EvidenceRequirements` it never answered. The read path asserts that rule; it does not claim to detect death |
| A14-10 | `cargo test -p agent-desktop-windows --lib` | n/a | api-contract | 2.1's `ensure_owned_process_mta_and_dpi` guards `CoInitializeEx` behind a process-wide `OnceLock`, with its own doc-comment recording that this is sound "only because the CLI calls this once from its main thread before any COM work" | the caveat is load-bearing and 2.2 is the first consumer to reach it. `CoInitializeEx` is thread-local while the guard is process-wide, so in a multi-threaded test binary exactly one thread joins the apartment and every other thread's `CoCreateInstance` returns `CO_E_NOTINITIALIZED`; observed as 17 of 62 tree tests failing under default test parallelism and 0 of 62 with `--test-threads=1` | NEW-EDGE | no live defect - the CLI is single-threaded at bootstrap and the cdylib already uses the process-wide `CoIncrementMTAUsage` path. 2.2's tests use `ensure_hosted_library_mta_and_dpi`, which is the semantically correct primitive for threads this product does not own. Recorded because the next consumer that reaches COM from a worker thread - Phase 5's daemon, or any 2.4+ code that walks off the main thread - hits the same wall, and the failure is a confusing `CO_E_NOTINITIALIZED` from a process that did bootstrap successfully |
| A14-11 | `cargo test -p agent-desktop-windows --lib -- tree::automation` | uia3-com | api-contract | 2.2's risk register records that UIA has no per-element messaging timeout, that `ConnectionTimeout` and `TransactionTimeout` are not documented to bound the `WM_GETOBJECT` `SendMessage` that `ElementFromHandle` issues, and that "whether a non-pumping target produces a clean timeout or a hang is unverified, and the fixture cannot produce the condition" | the fixture **can** produce it: `CreateWindowExW` dispatches `WM_CREATE` inline, so a thread can own a live, visible, non-zero-rect window and then never dispatch again. Against that window `ElementFromHandle` produces **neither** a clean timeout nor a bounded failure - it blocks. Measured by removing the mitigation below: the resolver did not return within the test's 30 s watchdog and the case took 59.21 s to fail, against 2.01 s with it. A `Deadline` checked before and after the call cannot interrupt it, because the block is inside the call | NEW-EDGE | the question is closed: it is a hang, not a timeout. `root_from_hwnd` now asks `SendMessageTimeoutW(WM_NULL, SMTO_ABORTIFHUNG)` first, so a target that is already hung becomes a structured `APP_UNRESPONSIVE`, and asks `IsWindow` before that so a destroyed handle stays `WINDOW_NOT_FOUND` per A14-5 rather than being reported as hung. This is a mitigation, not a guarantee - a target that stops pumping between the probe and the call still blocks, and bounding that needs the call issued on a thread the caller can abandon. 2.4 owns the snapshot path that would need it |
| A14-12 | `cargo test -p agent-desktop-windows --lib -- tree::automation` | uia3-com | api-contract | 2.2's risk register names `ConnectionTimeout` (2 s) and `TransactionTimeout` (20 s) as UIA's own bounds and records that they are "not documented to bound the `WM_GETOBJECT` `SendMessage` that `ElementFromHandle` issues" | the documentation question is moot, because the setters are **unreachable from this client**. `uiautomation` 0.25.0 constructs `CUIAutomation`, and on build 17763 that object returns `E_NOINTERFACE` (`0x80004002`) for `IUIAutomation2`, which is where `SetConnectionTimeout` and `SetTransactionTimeout` live. `CUIAutomation8` is the CLSID that carries them, and 2.0's own COM probe used it - the crate does not. Separately, `uiautomation::UIElement` is `!Send` (`NonNull<c_void>` is not `Send`), so a resolution issued on a thread the caller could abandon cannot return its element without an `unsafe impl Send` | NEW-EDGE | there is no UIA-level bound available behind `new_direct()`, and no thread-level bound available without the `unsafe impl Send` KTD2 forbids outright. 2.2 ships the bound that is reachable: probe the target with `SendMessageTimeoutW(WM_NULL, SMTO_ABORTIFHUNG)` before issuing a call that would block on it. A sub-phase that needs a true bound must either construct a `CUIAutomation8` client itself - reopening the `new_direct()` decision with evidence - or run the whole observation on an abandonable thread, which is a process-shape decision for 2.4 and Phase 5 rather than a resolver detail. A test pins the `E_NOINTERFACE`, so the day it changes, the cheaper bound becomes available loudly |
| A14-12 | `cargo test -p agent-desktop-windows --lib -- tree::automation` | uia3-com | api-contract | 2.2's risk register names `ConnectionTimeout` (2 s) and `TransactionTimeout` (20 s) as UIA's own bounds and records that they are "not documented to bound the `WM_GETOBJECT` `SendMessage` that `ElementFromHandle` issues" | **they do bound it** - the obstacle was reaching them, not their effect. `uiautomation::UIAutomation::new_direct()` is `CoCreateInstance(&CUIAutomation, ...)`, and on build 17763 that object returns `E_NOINTERFACE` (`0x80004002`) for `IUIAutomation2`, where the setters live, so calls through the crate's own client carry no timeout at all. Built instead from `CUIAutomation8` with `SetConnectionTimeout(2000)`, the identical call against a window whose thread owns it but never dispatches returns `UIA_E_TIMEOUT` (`0x80131505`) in **1.02 s**; through the crate's client the same call did not return inside a 30 s watchdog and the case took 59.09 s to fail | CONTRADICTS | the resolver's remaining hang was closable and is closed. The client is constructed from `CUIAutomation8` by direct `CoCreateInstance`, which keeps every property `new_direct()` was chosen for - it never calls `CoInitializeEx`, so it works inside an STA host and leaks no initialization count in a long-lived process - and differs only in CLSID, with a fallback to `new_direct()` wherever `CUIAutomation8` is unavailable. **This is a deviation from the Definition of Done's "constructed with `new_direct()` only" and is flagged for the owner rather than assumed**; KTD1's own prohibition is on `UIAutomation::new()`, which is still never called, and both of KTD1's stated reasons are preserved. A test pins that the crate's client lacks the interface, so if that changes the fallback becomes bounded too and says so |
## Session evidence (R6)