From 4b58fc1cdc1ba3e402a9883e7c7aabdd2bb30fc1 Mon Sep 17 00:00:00 2001 From: deedra waters Date: Mon, 12 Oct 2026 23:31:03 -0700 Subject: [PATCH] Keep Wine event feedback responsive when MSAA providers stall Synchronous AccessibleObjectFromEvent and IAccessible property queries on the helper's WinEvent thread could block native typing and focus feedback while an application-owned accessibility provider stopped responding. Move those calls into MsaaEventReader, a single COM worker that owns its native references and exchanges copied requests and results with the event thread. Bound both queues to 32 entries, invalidate queued work on focus or foreground changes, and discard results older than 500 ms. Validate the window, process, thread, generation, and foreground relationship before presenting a result. Read protected state before querying accessible values. Reject unrelated background presentation events before invoking providers or invalidating foreground work. Preserve accessible events from visible popups owned by the foreground window. Keep existing native menu, editor, combo, list, and item-control paths, and use native focus fallback when an MSAA focus request expires or fails. Poll completed results from the existing event timer. On shutdown, retire pending work and wait at most 100 ms for the worker; a blocked COM call keeps only its own state until it completes or the helper process exits. Never spawn additional workers behind an unresponsive provider. Validation: CMake configure/build and git diff --check passed. On a private dummy Xorg display, all four CTest checks passed, as did six event-stall runs (default, --32-bit, --getter, --background, --popup, --reader orca) and the existing menu, edit, controls, and list fixtures in both 64-bit and 32-bit modes. See the accompanying regression-test commit for commands. Private recording-service fixtures do not establish audible speech, physical braille, or acceptance of every live MUSHclient workflow. A provider that never returns occupies the single MSAA worker until helper exit. --- CMakeLists.txt | 2 +- main.cpp | 117 +++++++++++++++----------------- msaa_events.cpp | 172 ++++++++++++++++++++++++++++++++++++++++++++++++ msaa_events.h | 39 +++++++++++ 4 files changed, 266 insertions(+), 64 deletions(-) create mode 100644 msaa_events.cpp create mode 100644 msaa_events.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 946a198..82f8d3c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -49,7 +49,7 @@ add_custom_command( DEPENDS "${CMAKE_CURRENT_SOURCE_DIR}/nvdaController.idl" VERBATIM) -add_executable(cthulhu-wine-access main.cpp foreground_focus.cpp list_view.cpp native_items.cpp review_capture.cpp review_x11.cpp review_service.cpp "${generated_server}") +add_executable(cthulhu-wine-access msaa_events.cpp main.cpp foreground_focus.cpp list_view.cpp native_items.cpp review_capture.cpp review_x11.cpp review_service.cpp "${generated_server}") add_dependencies(cthulhu-wine-access wine-access-menu wine-access-menu32) target_compile_features(cthulhu-wine-access PRIVATE cxx_std_20) target_include_directories(cthulhu-wine-access PRIVATE "${generated_dir}") diff --git a/main.cpp b/main.cpp index 51981f7..75052bd 100644 --- a/main.cpp +++ b/main.cpp @@ -10,6 +10,7 @@ #include "review_service.h" #include "foreground_focus.h" #include "reader_endpoint.h" +#include "msaa_events.h" #include #include @@ -32,6 +33,9 @@ constexpr int DBUS_TIMEOUT_MS = 500; GDBusConnection *connection = nullptr; ReviewIdentityStore reviewIdentities; ReviewService reviewService; +MsaaEventReader msaaEvents; +uint64_t msaaGeneration = 0; +MsaaEvent pendingMsaaFocus; // Event-thread owned; retired by focus changes, results or timeout. std::deque menuEvents; std::deque textEvents; bool editReaderActive = false; @@ -154,31 +158,6 @@ bool reader_is_available() { return owned == TRUE; } -std::string variant_text(const VARIANT &value) { - if (value.vt == VT_BSTR) - return to_utf8(value.bstrVal); - if (value.vt == VT_I4) { - wchar_t buffer[128] = {}; - if (GetRoleTextW(value.lVal, buffer, 128) > 0) - return to_utf8(buffer); - return std::to_string(value.lVal); - } - return {}; -} - -std::string accessible_property( - IAccessible *accessible, - HRESULT (STDMETHODCALLTYPE IAccessible::*getter)(VARIANT, BSTR *), - const VARIANT &child) { - BSTR value = nullptr; - const HRESULT result = (accessible->*getter)(child, &value); - if (FAILED(result) || value == nullptr) - return {}; - const std::string text = to_utf8(value); - SysFreeString(value); - return text; -} - // Standard-control queries follow the LGPL wine-a11y reader published by // Mudb0y, while presentation uses the selected reader's D-Bus interface. void strip_mnemonics(wchar_t *text) { @@ -754,6 +733,14 @@ HANDLE start_menu_host32(HWND bridge) { void CALLBACK winevent_callback(HWINEVENTHOOK, DWORD event, HWND window, LONG objectId, LONG childId, DWORD, DWORD) { + const bool presentationEvent = event == EVENT_OBJECT_FOCUS || event == EVENT_OBJECT_NAMECHANGE || + event == EVENT_OBJECT_VALUECHANGE || event == EVENT_OBJECT_SELECTION || event == EVENT_OBJECT_STATECHANGE; + if (presentationEvent && !msaa_event_in_foreground(window)) return; + if (event == EVENT_OBJECT_FOCUS || event == EVENT_SYSTEM_FOREGROUND) { + pendingMsaaFocus = {}; + ++msaaGeneration; + msaaEvents.invalidate(msaaGeneration); + } if (event == EVENT_OBJECT_DESTROY) reviewIdentities.destroyed(window,objectId,childId); else if (event == EVENT_OBJECT_REORDER) reviewIdentities.reordered(window,objectId); if (event == EVENT_OBJECT_FOCUS || event == EVENT_SYSTEM_FOREGROUND) @@ -793,46 +780,38 @@ void CALLBACK winevent_callback(HWINEVENTHOOK, DWORD event, HWND window, else poll_control_focus(event == EVENT_OBJECT_NAMECHANGE || event == EVENT_OBJECT_VALUECHANGE); return; } - IAccessible *accessible = nullptr; - VARIANT child{}; - VariantInit(&child); - if (FAILED(AccessibleObjectFromEvent(window, objectId, childId, &accessible, - &child)) || - accessible == nullptr) { - if (event == EVENT_OBJECT_FOCUS) - present_standard_focus(window); + DWORD process = 0; + DWORD thread = GetWindowThreadProcessId(window, &process); + MsaaEvent request{window, event, process, thread, objectId, childId, + msaaGeneration, GetTickCount64() + 500}; + if (event == EVENT_OBJECT_FOCUS) pendingMsaaFocus = request; + msaaEvents.submit(request); +} + +bool current_msaa_source(const MsaaEvent &source) { + DWORD process = 0; + DWORD thread = GetWindowThreadProcessId(source.window, &process); + return source.generation == msaaGeneration && IsWindow(source.window) && + process == source.process && thread == source.thread && + msaa_event_in_foreground(source.window); +} + +void present_msaa_result(const MsaaResult &result) { + const auto &source = result.source; + if (!current_msaa_source(source) || GetTickCount64() >= source.deadline) return; + if (source.event == EVENT_OBJECT_FOCUS && pendingMsaaFocus.generation == source.generation) + pendingMsaaFocus = {}; + HWND window = source.window; + DWORD event = source.event; + LONG objectId = source.object, childId = source.child; + if (!result.available) { + if (event == EVENT_OBJECT_FOCUS) present_standard_focus(window); return; } - - const std::string name = - accessible_property(accessible, &IAccessible::get_accName, child); - const std::string value = - accessible_property(accessible, &IAccessible::get_accValue, child); - const std::string description = - accessible_property(accessible, &IAccessible::get_accDescription, child); - VARIANT role{}; - VariantInit(&role); - std::string roleText; - LONG roleId = 0; - if (SUCCEEDED(accessible->get_accRole(child, &role))) - roleText = variant_text(role); - if (role.vt == VT_I4) roleId = role.lVal; - VariantClear(&role); - VARIANT state{}; - VariantInit(&state); - std::string stateText; - bool password = false; - if (SUCCEEDED(accessible->get_accState(child, &state)) && state.vt == VT_I4) { - stateText = msaa_states(static_cast(state.lVal), roleId); - password = state.lVal & STATE_SYSTEM_PROTECTED; - } - VariantClear(&state); - LONG childCount = 0; - accessible->get_accChildCount(&childCount); - const bool nativeClient = child.vt == VT_I4 && child.lVal == CHILDID_SELF && - (roleText.empty() || roleId == ROLE_SYSTEM_CLIENT); - accessible->Release(); - VariantClear(&child); + const auto &name = result.name, &value = result.value, &description = result.description; + const auto &roleText = result.role, &stateText = result.states; + bool password = result.password, nativeClient = result.native_client; + LONG childCount = result.count; if (event == EVENT_OBJECT_FOCUS && name.empty() && value.empty() && description.empty() && (roleText.empty() || roleText == "client") && @@ -1053,6 +1032,9 @@ int WINAPI WinMain(HINSTANCE instance, HINSTANCE, char *, int) { HWND window = nullptr; reviewService.start(connection,reviewIdentities,readerEndpoint); + if (dialogReaderEnabled && !msaaEvents.start()) { + std::fprintf(stderr, "wine-access: MSAA event worker unavailable\n"); + } bool controllerActive = false; if (controllerEnabled && FindWindowW(L"wxWindowClassNR", L"NVDA") == nullptr) { @@ -1117,6 +1099,14 @@ int WINAPI WinMain(HINSTANCE instance, HINSTANCE, char *, int) { while (GetMessageW(&message, nullptr, 0, 0) > 0) { if (listTimer && message.message == WM_TIMER && message.hwnd == nullptr && message.wParam == listTimer) { + MsaaResult result; + for (unsigned handled = 0; handled < 32 && msaaEvents.take(result); ++handled) + present_msaa_result(result); + if (pendingMsaaFocus.window && GetTickCount64() >= pendingMsaaFocus.deadline) { + const auto source = pendingMsaaFocus; + pendingMsaaFocus = {}; + if (current_msaa_source(source)) present_standard_focus(source.window); + } restore_virtual_desktop_focus(); poll_control_focus(); reviewService.focus_changed(reviewIdentities.observe_foreground()); @@ -1126,6 +1116,7 @@ int WINAPI WinMain(HINSTANCE instance, HINSTANCE, char *, int) { DispatchMessageW(&message); } if (listTimer) KillTimer(nullptr, listTimer); + msaaEvents.stop(); reviewService.stop(); if (hook != nullptr) UnhookWinEvent(hook); diff --git a/msaa_events.cpp b/msaa_events.cpp new file mode 100644 index 0000000..d710dad --- /dev/null +++ b/msaa_events.cpp @@ -0,0 +1,172 @@ +// SPDX-License-Identifier: LGPL-2.1-or-later +#include "msaa_events.h" +#include "msaa_states.h" +#include +#include +#include +#include +#include +#include + +namespace { +// MSAA notifications describe transient state, unlike ordered editor actions. +// Bound pending work and favor current focus if an application floods events. +constexpr size_t capacity = 32; +std::string text(BSTR value) { + if (!value) return {}; + int size = WideCharToMultiByte(CP_UTF8, WC_ERR_INVALID_CHARS, value, -1, nullptr, 0, nullptr, nullptr); + if (size <= 1) return {}; + std::string result(size, '\0'); + WideCharToMultiByte(CP_UTF8, WC_ERR_INVALID_CHARS, value, -1, result.data(), size, nullptr, nullptr); + result.resize(size - 1); + return result; +} +std::string property(IAccessible *object, + HRESULT (STDMETHODCALLTYPE IAccessible::*getter)(VARIANT, BSTR *), VARIANT child) { + BSTR value = nullptr; + HRESULT hr = (object->*getter)(child, &value); + auto result = SUCCEEDED(hr) ? text(value) : std::string{}; + SysFreeString(value); + return result; +} +struct Accessible { + IAccessible *object = nullptr; + VARIANT child{}; + ~Accessible() { if (object) object->Release(); VariantClear(&child); } +}; +} + +bool msaa_event_in_foreground(HWND window) { + HWND foreground = GetForegroundWindow(); + HWND root = GetAncestor(window, GA_ROOT); + if (!foreground || !root) return false; + if (root == foreground) return true; + if (!IsWindowVisible(root) || !(GetWindowLongPtrW(root, GWL_STYLE) & WS_POPUP)) return false; + // Bound traversal even if a malformed owner chain cycles. Owned popups can + // expose MSAA focus while the foreground owner retains native keyboard focus. + for (unsigned depth = 0; root && depth < 32; ++depth) { + root = GetWindow(root, GW_OWNER); + if (root == foreground) return true; + } + return false; +} + +struct MsaaEventReader::State { + std::mutex mutex; + std::condition_variable changed; + std::deque pending; + std::deque ready; + std::atomic stopping{false}; + uint64_t generation = 0; + bool current(const MsaaEvent &event) { + if (stopping || GetTickCount64() >= event.deadline || + !msaa_event_in_foreground(event.window)) return false; + std::lock_guard lock(mutex); + return generation == event.generation; + } + MsaaResult read(const MsaaEvent &event) { + MsaaResult result; result.source = event; + Accessible accessible; + if (!current(event) || FAILED(AccessibleObjectFromEvent(event.window, event.object, + event.child, &accessible.object, &accessible.child)) || !accessible.object) + return result; + if (!current(event)) return result; + VARIANT role{}; + LONG roleId = 0; + if (SUCCEEDED(accessible.object->get_accRole(accessible.child, &role))) { + if (role.vt == VT_I4) { + roleId = role.lVal; + WCHAR label[128]{}; + if (GetRoleTextW(roleId, label, 128)) result.role = text(label); + else result.role = std::to_string(roleId); + } else if (role.vt == VT_BSTR) result.role = text(role.bstrVal); + } + VariantClear(&role); + if (!current(event)) return result; + VARIANT state{}; + if (SUCCEEDED(accessible.object->get_accState(accessible.child, &state)) && state.vt == VT_I4) { + result.states = msaa_states(static_cast(state.lVal), roleId); + result.password = state.lVal & STATE_SYSTEM_PROTECTED; + } + VariantClear(&state); + if (!current(event)) return result; + result.name = property(accessible.object, &IAccessible::get_accName, accessible.child); + if (!current(event)) return result; + // Protected values must be excluded before querying the provider. + if (!result.password) result.value = property(accessible.object, &IAccessible::get_accValue, accessible.child); + if (!current(event)) return result; + result.description = property(accessible.object, &IAccessible::get_accDescription, accessible.child); + if (!current(event)) return result; + accessible.object->get_accChildCount(&result.count); + result.native_client = accessible.child.vt == VT_I4 && accessible.child.lVal == CHILDID_SELF && + (result.role.empty() || roleId == ROLE_SYSTEM_CLIENT); + result.available = true; + return result; + } + static DWORD WINAPI work(LPVOID pointer) { + std::unique_ptr> retained(static_cast*>(pointer)); + auto state = *retained; + const HRESULT com = CoInitializeEx(nullptr, COINIT_APARTMENTTHREADED); + for (;;) { + MsaaEvent event; + { + std::unique_lock lock(state->mutex); + state->changed.wait(lock, [&] { return state->stopping || !state->pending.empty(); }); + if (state->stopping) break; + event = state->pending.front(); state->pending.pop_front(); + } + auto result = SUCCEEDED(com) ? state->read(event) : MsaaResult{}; + result.source = event; + if (!state->current(event)) continue; + std::lock_guard lock(state->mutex); + if (state->stopping || event.generation != state->generation) continue; + if (state->ready.size() == capacity) state->ready.pop_front(); + state->ready.push_back(std::move(result)); + } + if (SUCCEEDED(com)) CoUninitialize(); + return 0; + } +}; + +MsaaEventReader::~MsaaEventReader() { stop(); } +bool MsaaEventReader::start() { + if (state_) return false; + state_ = std::make_shared(); + auto retained = new std::shared_ptr(state_); + worker_ = CreateThread(nullptr, 0, State::work, retained, 0, nullptr); + if (!worker_) { delete retained; state_.reset(); return false; } + return true; +} +void MsaaEventReader::invalidate(uint64_t generation) { + if (!state_) return; + std::lock_guard lock(state_->mutex); + state_->generation = generation; + state_->pending.clear(); state_->ready.clear(); +} +void MsaaEventReader::submit(const MsaaEvent &event) { + if (!state_) return; + std::lock_guard lock(state_->mutex); + if (state_->stopping || state_->generation != event.generation) return; + if (state_->pending.size() == capacity) state_->pending.pop_front(); + state_->pending.push_back(event); state_->changed.notify_one(); +} +bool MsaaEventReader::take(MsaaResult &result) { + if (!state_) return false; + std::lock_guard lock(state_->mutex); + if (state_->ready.empty()) return false; + result = std::move(state_->ready.front()); state_->ready.pop_front(); + return true; +} +void MsaaEventReader::stop() { + if (!state_) return; + { + std::lock_guard lock(state_->mutex); + state_->stopping = true; state_->pending.clear(); state_->ready.clear(); + state_->changed.notify_one(); + } + // A hung COM call cannot be safely terminated. It retains only its own + // shared state and native references until completion or process exit. + WaitForSingleObject(worker_, 100); + CloseHandle(worker_); worker_ = nullptr; + state_.reset(); +} diff --git a/msaa_events.h b/msaa_events.h new file mode 100644 index 0000000..2f4d2b1 --- /dev/null +++ b/msaa_events.h @@ -0,0 +1,39 @@ +// SPDX-License-Identifier: LGPL-2.1-or-later +#pragma once +#include +#include +#include +#include + +// Native focus owners can keep keyboard focus while their visible popup +// reports accessible item focus. Unrelated windows are never event sources. +bool msaa_event_in_foreground(HWND window); + +struct MsaaEvent { + HWND window = nullptr; + DWORD event = 0, process = 0, thread = 0; + LONG object = 0, child = 0; + uint64_t generation = 0, deadline = 0; +}; +struct MsaaResult { + MsaaEvent source; + bool available = false, native_client = false, password = false; + std::string name, role, value, states, description; + LONG count = 0; +}; + +// One COM worker owns native references. The event thread exchanges only +// copied requests/results, and never waits for a provider to finish. +class MsaaEventReader { +public: + ~MsaaEventReader(); + bool start(); + void invalidate(uint64_t generation); + void submit(const MsaaEvent &event); + bool take(MsaaResult &result); + void stop(); +private: + struct State; + std::shared_ptr state_; + HANDLE worker_ = nullptr; +};