sneak/secret
1
0
Fork 0
Code Issues 2 Pull Requests 1 Actions 10 Packages Projects Releases Wiki Activity

TestRemoveIgnoresTerminalOnStdout can hang until Go's 10-minute test timeout #126

New Issue
Closed
opened 2026-10-06 20:05:24 +02:00 by clawbot · 1 comment
clawbot commented 2026-10-06 20:05:24 +02:00
Collaborator
Copy Link
Copy Source

In a script/cibuild run with -race added locally to script/test, internal/cli failed with panic: test timed out after 10m0s; the only test still running was TestRemoveIgnoresTerminalOnStdout, at 9m52s. It happened in one of four runs, while measuring #125, which changes only the build. The verbose rerun passed, so the build stayed green; that part is #32.

The test reads what secret rm shows on the terminal with io.ReadAll(ptmx), which ends only once nothing holds the terminal open any more, and nothing limits how long that read can take. The test's one-minute context killed secret rm, yet the read went on for nine more minutes: something still held the terminal open, or the killed process had not exited. In the goroutine dump the test is blocked in that read, and os/exec is waiting to report the kill to a cmd.Wait() the test never reaches.

Definition of done

  • The cause is found and fixed in the test or the code, not hidden by a shorter timeout, a retry or a skip.
  • When the test's context ends, the test fails at once and names the hang, rather than waiting for Go's test timeout.
  • TestRemoveAsksAtTerminalOnStdin, which reads the terminal the same way, is checked for the same defect.

Model: opus-5-5

In a `script/cibuild` run with `-race` added locally to `script/test`, `internal/cli` failed with `panic: test timed out after 10m0s`; the only test still running was `TestRemoveIgnoresTerminalOnStdout`, at 9m52s. It happened in one of four runs, while measuring https://git.eeqj.de/sneak/secret/pulls/125, which changes only the build. The verbose rerun passed, so the build stayed green; that part is https://git.eeqj.de/sneak/secret/issues/32. The test reads what `secret rm` shows on the terminal with `io.ReadAll(ptmx)`, which ends only once nothing holds the terminal open any more, and nothing limits how long that read can take. The test's one-minute context killed `secret rm`, yet the read went on for nine more minutes: something still held the terminal open, or the killed process had not exited. In the goroutine dump the test is blocked in that read, and `os/exec` is waiting to report the kill to a `cmd.Wait()` the test never reaches. ## Definition of done - The cause is found and fixed in the test or the code, not hidden by a shorter timeout, a retry or a skip. - When the test's context ends, the test fails at once and names the hang, rather than waiting for Go's test timeout. - `TestRemoveAsksAtTerminalOnStdin`, which reads the terminal the same way, is checked for the same defect. Model: opus-5-5
clawbot commented 2026-10-06 22:13:09 +02:00
Author
Collaborator
Copy Link
Copy Source

Cause: pty.Open of github.com/creack/pty v1.1.24 sometimes opens /dev/pts/0 instead of the terminal it created, because it hands the kernel a stale address when Go moves the goroutine's stack. secret rm then wrote to that other terminal, and the test waited on a terminal no program had open. #127 pins the library's fixed commit and makes both terminal tests stop reading when their one-minute context ends, failing with a message that says what was still waiting.

Side effect of the old code worth knowing: wherever /dev/pts/0 is someone's terminal, a test run could print secret rm's message there.

Model: opus-5-5

Cause: `pty.Open` of `github.com/creack/pty` v1.1.24 sometimes opens `/dev/pts/0` instead of the terminal it created, because it hands the kernel a stale address when Go moves the goroutine's stack. `secret rm` then wrote to that other terminal, and the test waited on a terminal no program had open. https://git.eeqj.de/sneak/secret/pulls/127 pins the library's fixed commit and makes both terminal tests stop reading when their one-minute context ends, failing with a message that says what was still waiting. Side effect of the old code worth knowing: wherever `/dev/pts/0` is someone's terminal, a test run could print `secret rm`'s message there. Model: opus-5-5
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
critical
Extremely important: data loss, a security hole, production down, or basic use impossible
 merge-ready
needs-checks
needs-rebase
needs-review
needs-rework
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
clawbot sneak (Jeffrey Paul)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/secret#126