Keep Wine editor feedback responsive during reader reply stalls

PresentAccessibleEvent previously waited synchronously for the reader on
Wine Access's message-loop thread. A slow D-Bus acknowledgment prevented
that thread from receiving subsequent WM_COPYDATA editor notifications.
The hooks time out after 50 ms, so feedback disappeared even though the
application accepted the input. With a 350 ms delayed focus reply, the
original helper inserted all of abbacdef but forwarded only e and f.

Move menu, editor and standard-control event delivery to one reader worker.
Keep a FIFO of at most 128 pending events, retain each variant and the bus
connection until delivery completes, and preserve focus-before-text order.
Keep the 500 ms reply deadline and cancel outstanding calls during normal
shutdown without making the event thread wait for the reader. Event-thread
success now means queued; controller RPC calls retain their synchronous
reader status behavior.

Retain the most recent focus dependency on the worker. After a transport
failure, re-establish that focus before sending dependent editor actions.
Treat a successful false reply as a completed exchange because filtering
can suppress presentation while still establishing focus. Never replay a
timed-out text action. Report transport failures, recovery and queue
overflow without including event contents in diagnostics.

Add a native editor fixture and a private-session D-Bus integration runner
for delayed replies, reply timeouts, failed initial focus, repeated typing,
event order, unchanged native text/caret, and shutdown with a withheld
reply. Wait for editor-hook attachment before triggering typing so the
32-bit companion is exercised reliably. Document queue behavior, limits,
regression commands and remaining live acceptance checks.

Validation performed sequentially on a private Xorg dummy display with
disposable Wine prefixes and recording reader services:

- cmake --build build --parallel
- python3 tests/reader_stall_integration.py --build-dir build
- The reader-stall check with --32-bit, --timeout, --focus-error, and
  --32-bit --focus-error
- ctest --test-dir build --output-on-failure: all four checks passed
- python3 tests/menu_integration.py --build-dir build with --fixture menu,
  edit, edit --32-bit, edit --restart-helper, notepad, controls, and
  list --32-bit
- python3 tests/event_stall_integration.py --build-dir build
- python3 tests/controller_integration.py --build-dir build
- git diff --check

The delayed-reply regression failed against the original synchronous
helper and passed after the worker change. The failed-focus scenario also
failed before dependency recovery was added and passed afterward.

These recordings establish the shared failure paths, not the cause of
every live MUSHclient pause or audible speech and physical braille
acceptance. Live checks remain for command typing, incoming plugin speech,
output-buffer arrows, menus, switching between Wine and Linux, reader
restart, and protected fields. An unavailable reader can still cause
feedback loss or bounded queue overflow.

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}")

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

main.cpp

@@ -11,6 +11,7 @@
 #include "foreground_focus.h"
 #include "reader_endpoint.h"
 #include "msaa_events.h"
+#include "reader_events.h"
 
 #include <commctrl.h>
 #include <gio/gio.h>
@@ -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<WineAccessMenuEvent> 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",
+  return readerEvents.submit(g_variant_new(
-                      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)

reader_events.cpp

@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: LGPL-2.1-or-later
+#include "reader_events.h"
+#include <condition_variable>
+#include <cstdio>
+#include <deque>
+#include <mutex>
+#include <string>
+
+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<GVariant *> 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<std::shared_ptr<State>> retained(
+        static_cast<std::shared_ptr<State> *>(pointer));
+    auto state = *retained;
+    unsigned failures = 0;
+    std::unique_ptr<GVariant, decltype(&g_variant_unref)> focus(nullptr, g_variant_unref);
+    std::string focusSource;
+    bool focusDelivered = false;
+    for (;;) {
+      GVariant *parameters;
+      {
+        std::unique_lock<std::mutex> 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<State>(connection, endpoint);
+  auto retained = new std::shared_ptr<State>(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<std::mutex> 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<std::mutex> 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();
+}

reader_events.h

@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: LGPL-2.1-or-later
+#pragma once
+#include "reader_endpoint.h"
+#include <gio/gio.h>
+#ifndef NOMINMAX
+#define NOMINMAX
+#endif
+#include <windows.h>
+#include <memory>
+
+// 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> state_;
+  HANDLE worker_ = nullptr;
+};

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`:

tests/reader_stall_fixture.c

@@ -0,0 +1,66 @@
+// SPDX-License-Identifier: LGPL-2.1-or-later
+#include <windows.h>
+#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;
+}

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 = '<node><interface name="' + interface + '"><method name="PresentAccessibleEvent">'
+xml += ''.join('<arg type="' + kind + '" direction="in"/>' for kind in ["s"] * 7 + ["i", "i", "s"])
+xml += '<arg type="b" direction="out"/></method></interface></node>'
+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)