Skip to content

tests: session_lifecycle types each key once the panel shows the last - #1169

Merged
Japabu merged 3 commits into
mainfrom
ps2-pace
Nov 4, 2026
Merged

Japabu merged 3 commits into
mainfrom
ps2-pace

Conversation

@Japabu

@Japabu Japabu commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

session_lifecycle typed detach /system/bin/hangupholder as 32 key presses back to back: 64
set-1 bytes into QEMU's PS/2 queue, which holds 16 and drops the rest without a word. A guest the
host starves drains it late, so on loaded hosts the shell got detach /pholder (#1057) or
/sysngupholder (#1150), and the row stalled waiting for the holder
(issues/a-command-typed-through-qmp-arrives-truncated-under-host-load.md).

Where the keys drop

Two stagings with scratch hooks, neither committed:

  • The whole line as one input-send-event, on the dev host (before the rule moved x86 guests to
    jan-pc). QEMU queues the line before any vCPU reads port 0x60, so this is the worst case of a
    starved guest. ps2server, saying its counters every wake, read 20 bytes: Ctrl+N's 4 and 16 of the
    line's 64, every one decoded (bytes = keys, 0 undecoded, 0 overruns). The shell got a truncated
    line.
  • One QMP call per key on an idle host: all 68 bytes arrived.

The guest loses nothing it is given; the harness typed past the device.

Change

The line is typed one key at a time, each once the screen's top half has changed since the key
before. Every key, a space included, moves the terminal's inverse-cell cursor, so at most one key's
press and release are in the queue. Only the top half is compared: Ctrl+N's terminal is the only
window and sits at the top-left, while the taskbar's readout at the bottom redraws every second
whatever is typed and would acknowledge a key the guest never saw. Before the first key the frame
must hold still once, so the window's own first paint is not taken for a key. A key the panel never
shows within 10 s (a liveness ceiling) reds by name: "the panel never changed after {key} of
{text} was typed".

Evidence

On jan-pc (KVM), through qemu-remote.sh:

  • session_lifecycle at a0fb2d9e2, this head: PASS, exit 0 (r1 remote-ps2-r1.log). At
    3610f2d50, round 0's version: PASS (r0 remote-ps2.log.gz).
  • Negative control at d0dc31069 (scratch branch, this head with the tenth key, y, never
    pressed): FAIL, exit 1, "the panel never changed after "y" of "detach /system/bin/hangupholder" was
    typed", 16 s in. The taskbar's clock ticked during that wait, so the comparison ignores it.
  • A scratch revision overflowed the queue on purpose first (62 key events in one batch), then pressed
    Enter and typed the line paced: PASS. 132 key events had been typed up to the holder's spawn and
    ps2server had read 86 bytes (4 + 16 + 2 + 64), so the batch was nearly all dropped by QEMU. The input path then went
    on and delivered the paced line whole. That is one boot of
    issues/the-console-input-path-can-stop-after-a-ps2-overflow.md's staging shape on the current
    userland ps2server, recorded there.

Logs: r0 https://github.com/ToyOSOrg/evidence/tree/5c1afc674a269ba801b3852a76f9818d46b4f80f/pr-1169/r0,
r1 https://github.com/ToyOSOrg/evidence/tree/51bd55331bf447c6e4dbdf6e5e1b34019934cc84/pr-1169/r1

Checks

  • cargo test --test toyos-build --no-run: exit 0. No host test reaches the typing; the guest runs
    above are its evidence.

🤖 Generated with Claude Code

Japabu and others added 2 commits November 4, 2026 09:00
session_lifecycle typed `detach /system/bin/hangupholder` as 32 key
presses back to back, 64 set-1 bytes, into QEMU's PS/2 queue, which
holds 16 and drops the rest without a word. A guest the host starves
drains it late, so on loaded hosts the shell got `detach /pholder` (#1057)
or `/sysngupholder` (#1150) and the row stalled waiting for the holder.

Where the keys drop, staged deterministically on the dev host: the whole
line sent as one `input-send-event`, so QEMU queues it before any vCPU
reads port 0x60. ps2server read 20 bytes in all: Ctrl+N's 4 and 16 of the
line's 64, with every byte it read decoded (bytes = keys, 0 undecoded,
0 overruns). The other 48 never reached the guest. Typed one QMP call
per key on an idle host, it read all 68. The guest loses nothing it is
given; the harness typed past the device.

Each key now goes once the panel has changed since the one before, so at
most one key's press and release, and a space's, are in the queue. A key
the panel never shows within a liveness ceiling reds by name.

`a-command-typed-through-qmp-arrives-truncated-under-host-load` is closed:
its exit asked where the keys drop and for the fix there.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The overflow-wedge record still described the kernel's i8042 ISR. One
session_lifecycle boot on jan-pc overflowed QEMU's queue on purpose and
the input path delivered the next line whole; recorded as one boot, which
the record's rate does not let decide anything.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Japabu

Japabu commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator Author

Review r0 of #1169 at f7569c4

Architecture. The diagnosis is sound, and the fix belongs in the harness. QEMU's 16-byte PS/2 queue drops bytes once it is full. In sl-batch.log ps2server read exactly 4 + 16 bytes, with 0 undecoded and 0 overruns. In sl-1.log it read all 68, and the run with the paced fix read every byte of the line. The guest decoded everything it was given. Pacing a host-side injector to the device's capacity is not hiding a guest defect. session_lifecycle is the only path that types several keys into PS/2. The other QmpInput users in tests/toyos.rs send one key (scale, memory_floor) or l s ret to a usb-kbd, which is not the PS/2 queue. So a local type_paced is enough, and a shared abstraction is not needed. Polling screendump_while with a scaled ceiling counts as waiting on an observable condition, not a fixed delay. The framebuffer offers no notification. The closed record has no citations left (git grep is empty), and its durable rule now sits at type_paced. Net change: +33/-7 in the harness, -34 and +8 in issues.

The problem is that the condition being waited on is not the event the code says it is.

BLOCKERS

  1. tests/common/sessionlifecycle.rs:123 — "the panel changed" includes the taskbar readout, which changes by itself. compositor::tick_taskbar (system/compositor/src/session.rs:1959) damages the taskbar status box once a second. render.rs:268-283 repaints it with CPU {n}% and the clock, so the pixels change whenever CPU% moves, and typing moves it. d.pixels != shown compares the whole 1920x1080 dump, so a taskbar tick satisfies the wait whether or not the key was drained.

    • The PR and commit say "a key the panel never shows within a liveness ceiling reds by name". That is false. A lost key is caught at the next second's tick, and the run then stalls later at "the shell to detach the holder", which is the diagnostic this change was meant to replace.
    • It also weakens the invariant the fix depends on ("at most one key's press and release ... are in the queue"). Under starvation, the case this targets, the compositor can tick while ps2server has not run, so the harness types ahead again.
    • The idle-host PASS cannot show this: 32 keys took 0.72 s (remote-ps2.log 2.073 → 2.795), well under one tick.
    • Fix: compare only what the typed key can change. For example, compare the rows above the taskbar strip (Taskbar::strip, system/compositor/desktop/src/taskbar.rs:30, bottom of the screen), or just the terminal window. Then the error path actually reports a key that was never shown.
  2. tests/common/sessionlifecycle.rs:115,120-122 — the spc special case and "A space may draw nothing" are false by inspection. The terminal draws its cursor as an inverse cell, on by default (system/terminal/src/console.rs:230-235). Nothing in the shell or terminal sends ?25l. A space moves that cell, so the panel changes like it does for any other key. Delete the branch and the comment sentence, so that every key is waited for and the "one key in the queue" claim holds without exceptions. (This is only true once blocker 1 is fixed. Until then everything "changes".)

  3. issues/the-console-input-path-can-stop-after-a-ps2-overflow.md:78-79 — the recorded counts are wrong. "ps2server read 78 bytes of the 132 typed up to the holder's spawn" uses the last counter line printed before the spawn line (remote-scratch.log:737). By guest timestamp, ps2server had read 86 bytes at 2.656, before the spawn at 2.657 (lines 739-746 are written after it). 86 = 4 (Ctrl+N) + 16 (one queue of the 62-event batch) + 2 (Enter) + 64 (the paced line), which is exactly what a single overflow predicts. 78 would mean that only 8 of the batch survived, and nothing explains that. A record must not carry a misread number. Correct it to 86, or state just what it shows: the batch kept one queue's 16 bytes, and every later byte arrived. While you are editing it, the paragraph could be shorter (see suggestion 2).

SUGGESTIONS

  1. tests/common/qemu.rs QmpInput::keys: add one line on its doc saying QEMU's PS/2 queue holds 16 bytes and drops the rest, so a caller typing more than that must pace. The rule currently lives only in a private helper in one test module, and the next multi-key typer would not see it.
  2. The wedge record's new paragraph spends a sentence saying that it "establishes nothing either way". Two sentences are enough: the i8042 is now ps2server's, and the kernel ISR and drain counter it describes are gone. One overflowed boot on 2026-11-04 kept one queue and took every later byte (evidence link).
  3. shown is captured right after terminal: ready. If the terminal's first frame has not been composited yet, the first "change" is the window appearing, not d arriving. This is bounded (4 bytes) and harmless, but awaiting one stable dump before the first key would make the invariant exact.

Evidence

SEND BACK

…'s clock

- The comparison was the whole screen, and the taskbar's readout redraws
  every second, so a tick answered for a key that never arrived. Only the
  screen's top half is compared now: the terminal is the only window, at
  the top-left margin, and the taskbar is at the bottom.
- A space moves the terminal's inverse-cell cursor like any key, so its
  exemption goes and every key is waited for.
- The first key waits for one frame seen twice, so the window's own first
  paint is not taken for it.
- `QmpInput::keys` says that QEMU's PS/2 queue holds 16 bytes.
- The wedge record's count was read off the last line printed before the
  spawn; by guest timestamps it is 86, 4 + 16 + 2 + 64, one queue of the
  batch kept and every later byte taken.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Japabu

Japabu commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator Author

Review r1 of #1169 at a0fb2d9

Architecture

Unchanged from r0. The harness paces a host-side injector to QEMU's 16-byte PS/2 queue, using a local type_paced helper. This round fixes the condition it waits on.

Previous blockers

  1. Taskbar in the comparison: resolved. type_paced now compares pixels[..width*height/2], and Ppm::pixels is Vec<[u8; 3]>, so that is exactly the top half of the rows. I checked what that half contains:
    • Typed text is inside it. The terminal opens with Size::Any (system/terminal/src/main.rs:83), so initial_content places it at the margin (desktop/src/layout.rs:266-274). The run logs content=41,69 1838x938 on a 1920x1080 panel, which puts the shell's first row at y≈69, far above 540.
    • Every self-updating region is outside it. The only periodic damage in the compositor is tick_taskbar (session.rs:1959-1986), which damages only taskbar.status() inside strip(), the bottom chrome.taskbar rows. The terminal has no cursor blink: blink appears nowhere in system/terminal or the compositor. The terminal presents only on shell output, input or resize. The wallpaper is static, and nothing moves the pointer.
    • The control confirms both by measurement. remote-ps2-ctl.log at d0dc310 waited the whole scaled ceiling (16 s run) on the never-pressed y with the taskbar ticking, and reported "the panel never changed after "y"…". So nothing in the top half changed by itself for about 13 s.
  2. Space special case: resolved. The branch and its sentence are gone. The control adds evidence here too: key 6 is the space, and it was acknowledged before the run reached y at index 9.
  3. Wedge record count: resolved. The record now reads 86 = 4 + 16 + 2 + 64. I also adopted S2: the paragraph is now two sentences. The PR body matches.

S1 (the QmpInput::keys doc) is adopted.

Evidence

  • PASS at a0fb2d9: remote-ps2-r1.log records the revision line a0fb2d9e2 = a0fb2d9e218d…, PASS session_lifecycle, and rc=0.
  • Negative control: d0dc310 is this head plus control-skip-key.patch, which only skips press for index 9. It ends rc=1 with the expected message. This exercises the failure path that r0 found unreachable.

Findings

SUGGESTION S1. tests/common/sessionlifecycle.rs:123: the stillness wait does not establish that the first paint is done. The comment says the window's own first paint is not taken for a key. But two equal dumps 20 ms apart show only that the screen held still for one interval:

  • The terminal presents nothing before terminal: ready. Its first present follows the shell's startup prompt (system/shell/src/main.rs:137).
  • So a still frame of the empty window can satisfy the wait. The prompt's paint can then answer for d, which lets e follow while d is still queued.

That is at most 8 bytes, under the queue's 16, so nothing is lost. But the comment and the PR body's "at most one key's press and release" claim more than the code guarantees. A cheap fix is to word the comment as what it does: // Wait for one still frame first, so a paint in progress is not taken for a key. Another is to wait for the prompt instead.

LAND

@Japabu
Japabu marked this pull request as ready for review November 4, 2026 09:56
@Japabu
Japabu added this pull request to the merge queue Nov 4, 2026
Merged via the queue into main with commit eb9e05d Nov 4, 2026
10 checks passed
@Japabu
Japabu deleted the ps2-pace branch November 4, 2026 10:16
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.

1 participant