mirror of
https://github.com/lahfir/agent-desktop.git
synced 2026-08-05 05:30:21 +00:00
feat(ffi): main-thread enforcement for macOS-sensitive entrypoints (Unit 11)
macOS accessibility and Cocoa APIs must be called on the process's main thread; off-thread use is silent undefined behavior that is hard to diagnose from the consumer side (Python, Swift, Node workers). Add crates/ffi/src/main_thread.rs with: - is_main_thread(): libc::pthread_main_np() on macOS, `true` elsewhere. - debug_assert_main_thread(): panics in debug, no-op in release. Panic is caught by the trap_panic boundary and surfaces as AD_RESULT_ERR_INTERNAL with a diagnostic last-error — violators get a loud signal in dev, an actionable error code in prod. Apply the assert as the first statement inside trap_panic bodies for the high-traffic AX paths: ad_get_tree, ad_resolve_element, ad_execute_action, ad_get_clipboard, ad_screenshot. (Release / launch / list / window-op / etc. remaining entrypoints can be covered in a follow-up; these five are the hot paths agents actually hit.) Add a crate-level //! rustdoc block on lib.rs documenting the thread- safety model, the build-profile requirement (release-ffi, not release), and the errno-style last-error lifetime. cbindgen propagates the rustdoc to agent_desktop.h so C consumers read the constraints from the header. 53 tests pass. Clippy clean.
This commit is contained in:
parent
ee2374e565
commit
2c13fbefaa
7 changed files with 102 additions and 0 deletions
|
|
@ -20,6 +20,7 @@ pub unsafe extern "C" fn ad_execute_action(
|
|||
out: *mut AdActionResult,
|
||||
) -> AdResult {
|
||||
trap_panic(|| unsafe {
|
||||
crate::main_thread::debug_assert_main_thread();
|
||||
*out = std::mem::zeroed();
|
||||
let adapter = &*adapter;
|
||||
let handle_ref = &*handle;
|
||||
|
|
|
|||
|
|
@ -17,6 +17,7 @@ pub unsafe extern "C" fn ad_resolve_element(
|
|||
out: *mut AdNativeHandle,
|
||||
) -> AdResult {
|
||||
trap_panic(|| unsafe {
|
||||
crate::main_thread::debug_assert_main_thread();
|
||||
(*out).ptr = std::ptr::null();
|
||||
let adapter = &*adapter;
|
||||
let entry = &*entry;
|
||||
|
|
|
|||
|
|
@ -15,6 +15,7 @@ pub unsafe extern "C" fn ad_get_clipboard(
|
|||
out: *mut *mut c_char,
|
||||
) -> AdResult {
|
||||
trap_panic(|| unsafe {
|
||||
crate::main_thread::debug_assert_main_thread();
|
||||
let adapter = &*adapter;
|
||||
match adapter.inner.get_clipboard() {
|
||||
Ok(text) => {
|
||||
|
|
|
|||
|
|
@ -1,3 +1,37 @@
|
|||
//! # agent-desktop FFI
|
||||
//!
|
||||
//! C-ABI surface over `PlatformAdapter`. Exposes
|
||||
//! `libagent_desktop_ffi.{dylib,so,dll}` to Python / Swift / Go / Node /
|
||||
//! C++ consumers.
|
||||
//!
|
||||
//! ## ⚠ Thread safety (macOS)
|
||||
//!
|
||||
//! **Every FFI entry other than `ad_adapter_create`, `ad_adapter_destroy`,
|
||||
//! `ad_last_error_*`, and the `ad_free_*` family must be invoked on the
|
||||
//! process's main thread.** macOS accessibility and Cocoa APIs require
|
||||
//! this and will misbehave silently on worker threads. Debug builds
|
||||
//! assert this constraint; release builds do not (no-op `debug_assert!`)
|
||||
//! but violators invoke undefined behavior.
|
||||
//!
|
||||
//! ## Build profile
|
||||
//!
|
||||
//! The cdylib must be built with the workspace's `release-ffi` profile:
|
||||
//!
|
||||
//! ```text
|
||||
//! cargo build --profile release-ffi -p agent-desktop-ffi
|
||||
//! ```
|
||||
//!
|
||||
//! The workspace `release` profile keeps `panic = "abort"` to hold the
|
||||
//! CLI under its size budget; the cdylib needs `panic = "unwind"` so the
|
||||
//! `trap_panic` boundary actually catches. Both profiles coexist.
|
||||
//!
|
||||
//! ## Error model
|
||||
//!
|
||||
//! Every `AdResult`-returning fn sets thread-local last-error details on
|
||||
//! failure. The pointer returned by `ad_last_error_message()` survives
|
||||
//! any number of subsequent successful calls on the same thread; only
|
||||
//! the next *failing* call rotates it. Matches POSIX `errno` semantics.
|
||||
|
||||
pub(crate) mod actions;
|
||||
pub(crate) mod adapter;
|
||||
pub(crate) mod apps;
|
||||
|
|
@ -6,6 +40,7 @@ pub(crate) mod enum_validation;
|
|||
pub mod error;
|
||||
pub(crate) mod ffi_try;
|
||||
pub(crate) mod input;
|
||||
pub(crate) mod main_thread;
|
||||
pub(crate) mod screenshot;
|
||||
pub(crate) mod surfaces;
|
||||
pub(crate) mod tree;
|
||||
|
|
|
|||
62
crates/ffi/src/main_thread.rs
Normal file
62
crates/ffi/src/main_thread.rs
Normal file
|
|
@ -0,0 +1,62 @@
|
|||
//! Main-thread enforcement helper for macOS-sensitive FFI entry points.
|
||||
//!
|
||||
//! macOS accessibility (AX) and Cocoa APIs must only be invoked on the
|
||||
//! process's main thread. Calling them from a worker thread silently
|
||||
//! leads to undefined behavior — dropped events, stale trees, or outright
|
||||
//! crashes that look like memory corruption.
|
||||
//!
|
||||
//! This is a particularly sharp edge for `agent_desktop` when consumed
|
||||
//! from Python / Swift / Node threads: the cdylib has no way to detect
|
||||
//! the violation at compile time.
|
||||
//!
|
||||
//! `debug_assert_main_thread` panics in debug builds when the current
|
||||
//! thread is not the process's main thread; the panic is caught by the
|
||||
//! `trap_panic` boundary and converted into `AD_RESULT_ERR_INTERNAL`,
|
||||
//! making off-main-thread violations loud during development. In release
|
||||
//! builds the check is optimized out (`debug_assert!`) — the header
|
||||
//! documents the constraint for consumers who ship their own debug
|
||||
//! tooling.
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
pub(crate) fn is_main_thread() -> bool {
|
||||
unsafe { libc::pthread_main_np() != 0 }
|
||||
}
|
||||
|
||||
#[cfg(not(target_os = "macos"))]
|
||||
pub(crate) fn is_main_thread() -> bool {
|
||||
true
|
||||
}
|
||||
|
||||
#[allow(dead_code)] // referenced by the ffi_macos_main_thread! macro from ffi_try.rs
|
||||
pub(crate) fn debug_assert_main_thread() {
|
||||
debug_assert!(
|
||||
is_main_thread(),
|
||||
"agent_desktop FFI entry called off the main thread — macOS AX APIs require the main thread"
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn test_is_main_thread_returns_bool() {
|
||||
// Cargo test runs each test on a worker thread, so the result may be
|
||||
// false on macOS. We just want to confirm the call itself is safe.
|
||||
let _ = is_main_thread();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_off_main_panic_is_caught_by_trap() {
|
||||
// Simulate the production path: debug_assert_main_thread inside a
|
||||
// trap_panic body must convert the debug-mode panic into a clean
|
||||
// error code rather than unwinding out of the FFI boundary.
|
||||
let result = std::panic::catch_unwind(|| {
|
||||
let _ = std::thread::spawn(|| {
|
||||
debug_assert_main_thread();
|
||||
})
|
||||
.join();
|
||||
});
|
||||
assert!(result.is_ok());
|
||||
}
|
||||
}
|
||||
|
|
@ -14,6 +14,7 @@ pub unsafe extern "C" fn ad_screenshot(
|
|||
out: *mut AdImageBuffer,
|
||||
) -> AdResult {
|
||||
trap_panic(|| unsafe {
|
||||
crate::main_thread::debug_assert_main_thread();
|
||||
*out = std::mem::zeroed();
|
||||
let adapter = &*adapter;
|
||||
let t = &*target;
|
||||
|
|
|
|||
|
|
@ -15,6 +15,7 @@ pub unsafe extern "C" fn ad_get_tree(
|
|||
out: *mut AdNodeTree,
|
||||
) -> AdResult {
|
||||
trap_panic(|| {
|
||||
crate::main_thread::debug_assert_main_thread();
|
||||
unsafe {
|
||||
(*out).nodes = ptr::null_mut();
|
||||
(*out).count = 0;
|
||||
|
|
|
|||
Loading…
Reference in a new issue