diff --git a/CMakeLists.txt b/CMakeLists.txt index 82f8d3c..8624c31 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 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_executable(cthulhu-wine-access reader_events.cpp 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/README.md b/README.md index f3f40b7..38be2da 100644 --- a/README.md +++ b/README.md @@ -248,6 +248,16 @@ A provider that never completes still occupies the one MSAA worker until helper exit; native menus, editing, focus fallback and controller requests remain available. No extra workers accumulate behind blocked providers. +Reader acknowledgments also run on a separate worker. Menu, editor and control +events enter a bounded FIFO so a delayed D-Bus reply cannot block reception of +the hooks' `WM_COPYDATA` messages. The event thread's success means queued; +controller RPC requests still return the reader's actual status. At most 128 +events wait behind the active call. Each reply has a 500 ms timeout, and normal +shutdown cancels the call. Failed exchanges and queue overflow produce concise +stderr diagnostics without event text. After a failed exchange, editor actions +re-establish their focus dependency; timed-out text actions are never replayed. +An unavailable reader can still cause feedback loss or queue overflow. + Standard `SysListView32` controls now report the current row's text columns, selection and position, including 32-bit installers driven by the 64-bit diff --git a/main.cpp b/main.cpp index 75052bd..3dea3b2 100644 --- a/main.cpp +++ b/main.cpp @@ -11,6 +11,7 @@ #include "foreground_focus.h" #include "reader_endpoint.h" #include "msaa_events.h" +#include "reader_events.h" #include #include @@ -34,6 +35,7 @@ GDBusConnection *connection = nullptr; ReviewIdentityStore reviewIdentities; ReviewService reviewService; MsaaEventReader msaaEvents; +ReaderEventQueue readerEvents; uint64_t msaaGeneration = 0; MsaaEvent pendingMsaaFocus; // Event-thread owned; retired by focus changes, results or timeout. std::deque menuEvents; @@ -266,13 +268,12 @@ bool present_accessible_event(const std::string &sourceId, // Readers can update their focus owner even when they suppress its output. if (std::string(eventType) == "focus" && sourceId != source_id(lastTextControl, "text")) lastTextControl = nullptr; - return call_boolean("PresentAccessibleEvent", - g_variant_new( + return readerEvents.submit(g_variant_new( "(sssssssiis)", sourceId.c_str(), eventType, control.name.c_str(), control.role.c_str(), control.value.c_str(), control.states.c_str(), control.description.c_str(), control.position, - control.count, windowTitle.c_str())) == ERROR_SUCCESS; + control.count, windowTitle.c_str())); } ControlDescription describe_standard_control(HWND window) { @@ -1031,6 +1032,13 @@ int WINAPI WinMain(HINSTANCE instance, HINSTANCE, char *, int) { } HWND window = nullptr; + if (dialogReaderEnabled && !readerEvents.start(connection, readerEndpoint)) { + std::fprintf(stderr, "wine-access: reader event worker unavailable\n"); + CoUninitialize(); + g_object_unref(connection); + CloseHandle(singleton); + return 1; + } reviewService.start(connection,reviewIdentities,readerEndpoint); if (dialogReaderEnabled && !msaaEvents.start()) { std::fprintf(stderr, "wine-access: MSAA event worker unavailable\n"); @@ -1116,6 +1124,7 @@ int WINAPI WinMain(HINSTANCE instance, HINSTANCE, char *, int) { DispatchMessageW(&message); } if (listTimer) KillTimer(nullptr, listTimer); + readerEvents.stop(); msaaEvents.stop(); reviewService.stop(); if (hook != nullptr) diff --git a/reader_events.cpp b/reader_events.cpp new file mode 100644 index 0000000..5a42355 --- /dev/null +++ b/reader_events.cpp @@ -0,0 +1,136 @@ +// SPDX-License-Identifier: LGPL-2.1-or-later +#include "reader_events.h" +#include +#include +#include +#include +#include + +struct ReaderEventQueue::State { + State(GDBusConnection *bus, const ReaderEndpoint &reader) + : connection(G_DBUS_CONNECTION(g_object_ref(bus))), endpoint(reader), + calls(g_cancellable_new()) {} + ~State() { + clear(); + g_object_unref(calls); + g_object_unref(connection); + } + GDBusConnection *connection; + const ReaderEndpoint endpoint; + GCancellable *calls; + std::mutex mutex; + std::condition_variable changed; + std::deque pending; + bool stopping = false, overflowReported = false; + + void clear() { + for (auto *parameters : pending) g_variant_unref(parameters); + pending.clear(); + } + bool send(GVariant *parameters, unsigned &failures) { + GError *error = nullptr; + GVariant *reply = g_dbus_connection_call_sync( + connection, endpoint.busName, endpoint.objectPath, + endpoint.interfaceName, "PresentAccessibleEvent", parameters, + G_VARIANT_TYPE("(b)"), G_DBUS_CALL_FLAGS_NONE, 500, calls, &error); + // A false reply can simply mean filtered/duplicate feedback. Only a + // failed exchange warrants diagnostics. Never include event contents. + const bool delivered = reply != nullptr; + if (!delivered && !g_cancellable_is_cancelled(calls)) { + if (++failures == 1) + std::fprintf(stderr, "wine-access: reader event reply failed (domain %s, code %d)\n", + error ? g_quark_to_string(error->domain) : "unknown", error ? error->code : 0); + } else if (delivered && failures) { + std::fprintf(stderr, "wine-access: reader event replies resumed after %u failed calls\n", failures); + failures = 0; + } + if (reply) g_variant_unref(reply); + if (error) g_error_free(error); + return delivered; + } + static DWORD WINAPI work(LPVOID pointer) { + std::unique_ptr> retained( + static_cast *>(pointer)); + auto state = *retained; + unsigned failures = 0; + std::unique_ptr focus(nullptr, g_variant_unref); + std::string focusSource; + bool focusDelivered = false; + for (;;) { + GVariant *parameters; + { + std::unique_lock lock(state->mutex); + state->changed.wait(lock, [&] { return state->stopping || !state->pending.empty(); }); + if (state->stopping) break; + parameters = state->pending.front(); + state->pending.pop_front(); + } + const char *source = nullptr, *kind = nullptr; + g_variant_get_child(parameters, 0, "&s", &source); + g_variant_get_child(parameters, 1, "&s", &kind); + if (std::string(kind) == "focus") { + focus.reset(g_variant_ref(parameters)); + focusSource = source; + focusDelivered = state->send(parameters, failures); + } else { + // A lost focus exchange must not strand later editor actions. Replay + // only the focus dependency, never a timed-out ordered text action. + if (std::string(kind) == "text" && !focusDelivered && focus && focusSource == source) + focusDelivered = state->send(focus.get(), failures); + if (std::string(kind) != "text" || focusDelivered) { + if (!state->send(parameters, failures)) focusDelivered = false; + } + } + g_variant_unref(parameters); + } + return 0; + } +}; + +ReaderEventQueue::~ReaderEventQueue() { stop(); } + +bool ReaderEventQueue::start(GDBusConnection *connection, const ReaderEndpoint &endpoint) { + if (state_) return false; + state_ = std::make_shared(connection, endpoint); + 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; +} + +bool ReaderEventQueue::submit(GVariant *parameters) { + parameters = g_variant_ref_sink(parameters); + if (state_) { + std::lock_guard lock(state_->mutex); + // Keep memory bounded if the reader remains unavailable. Do not reorder + // actions or evict the focus on which queued editor actions depend. + if (!state_->stopping && state_->pending.size() < 128) { + state_->pending.push_back(parameters); + state_->changed.notify_one(); + return true; + } + if (!state_->stopping && !state_->overflowReported) { + std::fprintf(stderr, "wine-access: reader event queue full; feedback dropped\n"); + state_->overflowReported = true; + } + } + g_variant_unref(parameters); + return false; +} + +void ReaderEventQueue::stop() { + if (!state_) return; + { + std::lock_guard lock(state_->mutex); + state_->stopping = true; + state_->clear(); + state_->changed.notify_one(); + } + g_cancellable_cancel(state_->calls); + // The worker retains its connection and queue state if cancellation has + // not completed yet; it never references event-thread globals. + WaitForSingleObject(worker_, 100); + CloseHandle(worker_); + worker_ = nullptr; + state_.reset(); +} diff --git a/reader_events.h b/reader_events.h new file mode 100644 index 0000000..1557694 --- /dev/null +++ b/reader_events.h @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: LGPL-2.1-or-later +#pragma once +#include "reader_endpoint.h" +#include +#ifndef NOMINMAX +#define NOMINMAX +#endif +#include +#include + +// Event callbacks enqueue copied data. One worker preserves event order while +// waiting for reader acknowledgments, leaving WM_COPYDATA reception available. +class ReaderEventQueue { +public: + ~ReaderEventQueue(); + bool start(GDBusConnection *connection, const ReaderEndpoint &endpoint); + bool submit(GVariant *parameters); + void stop(); +private: + struct State; + std::shared_ptr state_; + HANDLE worker_ = nullptr; +}; diff --git a/tests/README.md b/tests/README.md index 308ebd1..e8d8255 100644 --- a/tests/README.md +++ b/tests/README.md @@ -1,5 +1,32 @@ # Wine control integration +## Reader reply stalls + +Run sequentially on a private X display: + +```sh +xvfb-run -a python3 tests/reader_stall_integration.py --build-dir build +xvfb-run -a python3 tests/reader_stall_integration.py --build-dir build --32-bit +xvfb-run -a python3 tests/reader_stall_integration.py --build-dir build --timeout +xvfb-run -a python3 tests/reader_stall_integration.py --build-dir build --focus-error +``` + +The recording reader delays an editor focus reply for 350 ms while its Wine +control types `abbacdef`. All eight actions, including repeated letters, must +arrive in order with focus established first, and native text and caret must +remain correct. `--timeout` extends the delay past the helper's D-Bus deadline; +`--focus-error` rejects the initial focus exchange and requires recovery before +later text. A second withheld reply checks normal helper shutdown. The fixture +waits for editor-hook attachment before typing, including the 32-bit host. + +The original synchronous helper dropped six of the eight characters during +the delayed-reply check. These private recordings establish that shared failure +path; they do not establish the cause of every live MUSHclient speech pause. +After installing the helper and restarting it, check command typing, incoming +plugin speech, output-buffer arrows, menus, return from Linux applications and +reader restart in MUSHclient. Check ordinary Notepad editing and password +exclusion too. Audible speech and physical braille need separate live checks. + ## MSAA event stalls Run sequentially on a private X display, for example with `xvfb-run -a`: diff --git a/tests/reader_stall_fixture.c b/tests/reader_stall_fixture.c new file mode 100644 index 0000000..3656f28 --- /dev/null +++ b/tests/reader_stall_fixture.c @@ -0,0 +1,66 @@ +// SPDX-License-Identifier: LGPL-2.1-or-later +#include +#include "../menu_bridge.h" + +static HWND editor; +static BOOL typed; + +static BOOL marker_exists(const WCHAR *variable) { + WCHAR path[4096]; + return GetEnvironmentVariableW(variable, path, 4096) && + GetFileAttributesW(path) != INVALID_FILE_ATTRIBUTES; +} + +static void marker(const WCHAR *variable) { + WCHAR path[4096]; + if (!GetEnvironmentVariableW(variable, path, 4096)) return; + HANDLE file = CreateFileW(path, GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, + FILE_ATTRIBUTE_NORMAL, NULL); + if (file != INVALID_HANDLE_VALUE) CloseHandle(file); +} + +static LRESULT CALLBACK window_proc(HWND window, UINT message, WPARAM wp, LPARAM lp) { + if (message == WM_TIMER && !typed && marker_exists(L"WINE_ACCESS_READER_WAITING")) { + typed = TRUE; + const WCHAR *text = L"abbacdef"; + for (const WCHAR *cursor = text; *cursor; ++cursor) + SendMessageW(editor, WM_CHAR, *cursor, 1); + WCHAR actual[32]; + DWORD start = 0, end = 0; + GetWindowTextW(editor, actual, 32); + SendMessageW(editor, EM_GETSEL, (WPARAM)&start, (LPARAM)&end); + if (lstrcmpW(actual, text) || start != 8 || end != 8) ExitProcess(2); + marker(L"WINE_ACCESS_READER_TYPED"); + return 0; + } + if (message == WM_TIMER && typed && marker_exists(L"WINE_ACCESS_READER_RESUME")) { + KillTimer(window, 1); + SendMessageW(editor, WM_CHAR, 'z', 1); + return 0; + } + if (message == WM_DESTROY) { PostQuitMessage(0); return 0; } + return DefWindowProcW(window, message, wp, lp); +} + +int WINAPI WinMain(HINSTANCE instance, HINSTANCE previous, LPSTR args, int show) { + (void)previous; (void)show; + if (lstrcmpA(args, "quit-helper") == 0) { + DWORD thread = GetWindowThreadProcessId(FindWindowW(WINE_ACCESS_MENU_CLASS, NULL), NULL); + return thread && PostThreadMessageW(thread, WM_QUIT, 0, 0) ? 0 : 3; + } + WNDCLASSW cls = {0}; + cls.hInstance = instance; cls.lpfnWndProc = window_proc; + cls.lpszClassName = L"WineReaderStallFixture"; + RegisterClassW(&cls); + HWND window = CreateWindowW(cls.lpszClassName, L"Reader stall fixture", + WS_OVERLAPPEDWINDOW, 100, 100, 500, 300, NULL, NULL, instance, NULL); + editor = CreateWindowW(L"Edit", L"", WS_CHILD | WS_VISIBLE | WS_TABSTOP, + 10, 10, 300, 30, window, NULL, instance, NULL); + ShowWindow(window, SW_SHOW); SetForegroundWindow(window); SetFocus(editor); + SetTimer(window, 1, 10, NULL); + MSG message; + while (GetMessageW(&message, NULL, 0, 0) > 0) { + TranslateMessage(&message); DispatchMessageW(&message); + } + return 0; +} diff --git a/tests/reader_stall_integration.py b/tests/reader_stall_integration.py new file mode 100644 index 0000000..a889355 --- /dev/null +++ b/tests/reader_stall_integration.py @@ -0,0 +1,124 @@ +"""Check ordered editor feedback while the reader delays or loses replies. + +Uses a private D-Bus service, disposable Wine prefix and private X display. +""" +import argparse +import os +from pathlib import Path +import subprocess +import sys +import tempfile +import time + +from gi.repository import Gio, GLib + +parser = argparse.ArgumentParser() +parser.add_argument("--build-dir", default="build") +parser.add_argument("--32-bit", dest="is32Bit", action="store_true") +parser.add_argument("--timeout", action="store_true") +parser.add_argument("--focus-error", action="store_true") +parser.add_argument("--session", action="store_true") +args = parser.parse_args() +build = Path(args.build_dir).resolve() +if not args.session: + sys.exit(subprocess.call(["dbus-run-session", "--", sys.executable, __file__, + "--session", "--build-dir", str(build), + *(["--32-bit"] if args.is32Bit else []), + *(["--timeout"] if args.timeout else []), + *(["--focus-error"] if args.focus_error else [])])) + +interface = "org.stormux.Cthulhu1.Service" +object_path = "/org/stormux/Cthulhu1/Service" +xml = '' +xml += ''.join('' for kind in ["s"] * 7 + ["i", "i", "s"]) +xml += '' +events, held = [], [] + + +def pump_until(predicate, seconds): + deadline = time.monotonic() + seconds + while time.monotonic() < deadline: + while GLib.MainContext.default().iteration(False): + pass + if predicate(): + return + time.sleep(.005) + raise AssertionError(f"condition timed out; events: {events}") + + +with tempfile.TemporaryDirectory(prefix="wine-reader-stall-") as temporary: + root = Path(temporary) + waiting, typed, resume = (root / name for name in ("waiting", "typed", "resume")) + env = os.environ.copy() + env.update(WINEPREFIX=str(root / "prefix"), WINEDEBUG="-all", + WINEDLLOVERRIDES="mscoree,mshtml=", CTHULHU_WINE_ACCESS_MANAGED="1") + for key, path in (("WAITING", waiting), ("TYPED", typed), ("RESUME", resume)): + env["WINE_ACCESS_READER_" + key] = "Z:" + str(path).replace("/", "\\") + + def record(connection, sender, path, iface, method, parameters, invocation): + event = parameters.unpack() + first_focus = not waiting.exists() and event[1] == "focus" and event[0].endswith(":text") + if first_focus and args.focus_error: + waiting.touch() + invocation.return_dbus_error("org.freedesktop.DBus.Error.Failed", "Reader temporarily unavailable") + return + events.append(event) + if first_focus: + held.append(invocation) + waiting.touch() + elif event[1] == "text" and event[2] == "z": + held.append(invocation) + else: + invocation.return_value(GLib.Variant("(b)", (True,))) + + connection = Gio.bus_get_sync(Gio.BusType.SESSION, None) + connection.register_object(object_path, Gio.DBusNodeInfo.new_for_xml(xml).interfaces[0], record, None, None) + connection.call_sync("org.freedesktop.DBus", "/org/freedesktop/DBus", "org.freedesktop.DBus", + "RequestName", GLib.Variant("(su)", (interface, 0)), None, + Gio.DBusCallFlags.NONE, 1000, None) + fixture = root / "fixture.exe" + compiler = "i686-w64-mingw32-gcc" if args.is32Bit else "x86_64-w64-mingw32-gcc" + subprocess.run([compiler, "-Wall", "-Wextra", "-mwindows", "-o", str(fixture), + str(Path(__file__).with_name("reader_stall_fixture.c")), "-luser32"], check=True) + children = [] + try: + subprocess.run(["wineboot", "-u"], env=env, check=True, timeout=45) + helper = subprocess.Popen([str(build / "cthulhu-wine-access.exe"), "--no-controller"], env=env) + children.append(helper) + time.sleep(1) + app = subprocess.Popen(["wine", str(fixture)], env=env) + children.append(app) + pump_until(waiting.exists, 5) + # Longer than the hook's 50 ms deadline, shorter than the bus timeout. + if not args.focus_error: + GLib.timeout_add(700 if args.timeout else 350, + lambda: (held.pop(0).return_value(GLib.Variant("(b)", (True,))), False)[1]) + pump_until(typed.exists, 3) + pump_until(lambda: len([e for e in events if e[1] == "text"]) >= 8, 3) + actions = [e[2] for e in events if e[1] == "text"] + assert actions == list("abbacdef"), f"editor feedback lost or reordered: {actions}" + focused = None + for event in events: + if event[1] == "focus": + focused = event[0] + elif event[1] == "text": + assert event[0] == focused, "text forwarded before its focus" + print("PASS: delayed reader preserves repeated typing, order, native text and caret", flush=True) + pump_until(lambda: not held, 2) + resume.touch() + pump_until(lambda: bool(held), 2) + started = time.monotonic() + quit_client = subprocess.Popen(["wine", str(fixture), "quit-helper"], env=env) + children.append(quit_client) + pump_until(lambda: helper.poll() is not None, .4) + assert helper.returncode == 0, f"helper shutdown failed: {helper.returncode}" + print(f"PASS: normal shutdown while reader withholds reply ({time.monotonic() - started:.3f} s)", flush=True) + finally: + for child in reversed(children): + if child.poll() is None: + child.terminate() + try: + child.wait(timeout=3) + except subprocess.TimeoutExpired: + child.kill(); child.wait(timeout=3) + subprocess.run(["wineserver", "-k"], env=env, timeout=5, check=False)