Skip to content

fix: stability batch - self-pipe busy loop, UIA walker, console flash, screenshot fallback, emoji crash - #397

Merged
Jeomon merged 8 commits into
mainfrom
fix/easy-issues
Aug 28, 2026
Merged

fix: stability batch - self-pipe busy loop, UIA walker, console flash, screenshot fallback, emoji crash#397
Jeomon merged 8 commits into
mainfrom
fix/easy-issues

Conversation

@Jeomon

@Jeomon Jeomon commented Aug 28, 2026

Copy link
Copy Markdown
Member

Six commits addressing reported stability bugs, each landing with regression tests. Full suite: 555 passed.

Closes #392, #382, #371, #369. Mitigates #301 / #385.


fix: rebuild the asyncio self-pipe instead of pinning a CPU core#392

asyncio wakes its event loop through a loopback socketpair. A Windows session or display transition tears that idle pair down, and BaseSelectorEventLoop._read_from_self reads the resulting EOF, hits its break, and returns without unregistering the socket. It stays permanently readable, so select() returns instantly on every pass and the server burns a full core — silently, with no exception and nothing in the log — until the process is killed.

Reproduced on this stack (CPython 3.14.3, _WindowsSelectorEventLoop), killing the write half on an otherwise idle loop:

           healthy:  cpu=0.00s  wall=3.01s  ->   0.0% of one core
  self-pipe closed:  cpu=2.98s  wall=3.00s  ->  99.5% of one core   (zero stderr)
   self-pipe reset:  cpu=2.89s  wall=3.00s  ->  96.3% of one core   (84k tracebacks in 3s)

The reporter filed this upstream as python/cpython#156333 against the proactor loop and proposed a guard on _loop_self_reading. That patch would be dead code here: serve has forced WindowsSelectorEventLoopPolicy since 757940d, so we reach the same dead end by a different path. He explicitly predicted the selector loop would spin too — the numbers above confirm it.

install_selfpipe_guard() wraps _read_from_self: after the stock method runs, it peeks the socket (BlockingIOError = healthy, immediate empty read = dead peer) and rebuilds a dead pipe through the loop's own _close_self_pipe() / _make_self_pipe(). ConnectionResetError from the abortive-close variant is handled the same way. Idempotent, no-op on a healthy loop.

Rebuilding is deliberately unbounded. An earlier draft capped it and unregistered the reader after 20 attempts; the test for that path proved it unsurvivable, since select() with no registered descriptors raises WinError 10022 on Windows and under the stdio transport the self-pipe is frequently the only descriptor. A WARNING at 20 rebuilds keeps a pathological machine visible instead.

Verification: the suite was run against stock asyncio with the guard stubbed out — 5 of 10 tests fail, the spin measuring 0.94 of a core. test_wakeups_survive_the_rebuild asserts a call_soon_threadsafe from another thread still wakes the loop afterwards, which the watchdog's focus callbacks depend on. The real server was also driven end to end (serve --transport stdio, MCP initialize round-trip in 1.94s, clean exit).

Worth noting for the future: WindowsSelectorEventLoopPolicy is deprecated and slated for removal in 3.16. When it goes, the default returns to the proactor loop and the upstream-reported path becomes live for us again.

feat: desktop service module with screen state management and UIA integration#301, #385

RawViewWalker.GetFirstChildElement returns NULL for every element on some configurations (Windows ARM64 under x86-emulated Python), while FindAll on the same element enumerates children normally — turning every window into an apparent leaf and emptying the accessibility tree. _ViewWalkerState cross-checks the first few empty walks with FindAll and latches onto whichever path works, so steady-state cost is one enumeration per element either way.

is_overlay_window also no longer treats "no children" alone as proof of an overlay; when child enumeration degrades, that heuristic filters out the entire desktop. A real app window always has a title, so both signals are now required.

These issues are hardware-specific and could not be reproduced on this machine, so the commit is a mitigation on reported symptoms rather than a confirmed fix — hence no closing keyword for either.

fix: stop PowerShell child processes flashing a console window#369

A console child started from a server with no console of its own gets a brand new console allocated: a window flashes and steals keyboard focus on every tool call. Redirecting the streams does not prevent the allocation — only CREATE_NO_WINDOW does. Applied to the child and to the taskkill fallbacks in the timeout path, composed with the existing CREATE_NEW_PROCESS_GROUP so the CTRL_BREAK_EVENT graceful-stop path is unaffected.

fix: fall back to mss when a screenshot backend returns an empty frame#371

On VM/RDP desktops the primary backend returns an undecodable frame after the first capture. Detect the empty frame and fall back rather than handing the caller a corrupt image. Reported against VM/RDP specifically, which is not reproducible here; the fallback logic itself is covered by tests.

fix: repair UTF-16 surrogate pairs before serializing the UI tree#382

Snapshot crashed with UnicodeEncodeError whenever the UI tree contained emoji. Lone surrogates are repaired before serialization.

fix: import Display and SemanticNode in the desktop service

Missing imports in desktop/service.py.


Testing: pytest — 555 passed. ruff check and ruff format --check clean on all touched files.

Jeomon added 6 commits August 18, 2026 08:07
get_state() builds a Display via _display_to_view(), and
_filter_semantic_node_to_region() builds a SemanticNode, but neither name
was imported. Both raise NameError on the main Snapshot path, so Snapshot
fails outright.

Display is defined in desktop/views.py and SemanticNode in tree/views.py.
Introduced by dc6d06c.
UIA returns UTF-16, so an astral character such as an emoji can arrive as
a raw surrogate pair rather than a single code point. Python holds those
surrogates happily, but the strict UTF-8 JSON encoder rejects them, so a
single emoji in an element name or window title failed the entire
Snapshot response with UnicodeEncodeError.

repair_surrogates() combines what can be paired and replaces unpaired
surrogates with U+FFFD, so a stray half-character costs one glyph instead
of the whole capture. It is applied next to the existing
remove_private_use_chars() step, and covers the window title strings as
well as the three tree renders.

Refs #382
The backend chain only fell through when a backend raised. On VM and RDP
guests dxcam instead initializes cleanly and then returns frames carrying
no pixel data, which travelled all the way to the client as an
undecodable image with no error in between. The only remedy was for the
user to find WINDOWS_MCP_SCREENSHOT_BACKEND and pin mss by hand.

Captures are now validated, and an unusable frame is treated exactly like
a raised failure so the chain moves on to mss. A backend caught doing
this is skipped for the rest of the process, since retrying it just
reproduces the same broken capture.

The check is deliberately structural only, never content: None, zero
dimensions, or pixel data that will not load. An all-black frame stays
valid, because a locked or screensaver desktop is a legitimate
screenshot.

Refs #371
The child was created with CREATE_NEW_PROCESS_GROUP but without
CREATE_NO_WINDOW. When the server has no console of its own, which is the
usual case running as an MCP extension host, Windows allocates a new
console for a console child. That window flashes and takes keyboard
focus, so keystrokes typed during a tool call are lost. Redirecting the
streams does not prevent the allocation; only the flag does.

CREATE_NO_WINDOW composes with the process-group flag, so CTRL_BREAK_EVENT
and the graceful-stop path are unaffected. The two taskkill fallbacks on
the timeout path get it too, since they flashed as well.

Diagnosis and patch from the reporter.

Refs #369
A Windows session or display transition tears down the loopback socketpair
asyncio uses to wake its event loop. BaseSelectorEventLoop._read_from_self
reads the resulting EOF, hits its break, and returns without unregistering
the socket. It stays permanently readable, so select() returns instantly on
every pass and the server burns a full core -- silently, with no exception
and no log line -- until the process is killed.

Wrap _read_from_self so a dead pipe is detected and rebuilt. Rebuilding is
deliberately unbounded: unregistering the reader would leave select() with
no descriptors, which raises WinError 10022 on Windows, and under the stdio
transport the self-pipe is often the only one. A warning at 20 rebuilds
keeps a pathological machine visible.

Upstream this is python/cpython#156333, reported there against the proactor
loop; the selector loop we run reaches the same dead end through
_read_from_self.

Fixes #392
# the strict UTF-8 JSON encoder reject the entire response, so every string
# heading into it is repaired -- window titles included, since an emoji in a
# window title is enough to take the whole snapshot down.
interactive_elements = repair_surrogates(interactive_elements)
# heading into it is repaired -- window titles included, since an emoji in a
# window title is enough to take the whole snapshot down.
interactive_elements = repair_surrogates(interactive_elements)
scrollable_elements = repair_surrogates(scrollable_elements)
Jeomon added 2 commits August 29, 2026 00:16
Resolves an import-formatting conflict in desktop/service.py: both sides import the same six names from desktop.views, main's single-line form kept.
test_get_state_region_takes_precedence_over_display mocks a two-display 3840x1080 desktop but parse_region_selection validates the region against the real virtual screen, so the test only passed on a machine wide enough to contain it. CI runs at 1024x768, where it has been failing on every push to main since #388 introduced it. Mock get_screen_box to match the displays the test already mocks.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Server pins a full CPU core after the screen wakes on Windows (asyncio self-pipe dies and the event loop busy-loops)

1 participant