Skip to content

test(lsp): wait for a killed server before reading its goroutine dump - #607

Open
devin-ai-integration[bot] wants to merge 1 commit into
developfrom
fix/lsp-silent-server-stderr
Open

devin-ai-integration[bot] wants to merge 1 commit into
developfrom
fix/lsp-silent-server-stderr

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

What and why

TestSilentServerFailsWithinDeadline (cmd/sysml-lsp/lifecycle_test.go) fails at random in the race job:

--- FAIL: TestSilentServerFailsWithinDeadline (0.51s)
    lifecycle_test.go:365: failure = "read header: EOF (server killed after 508ms of silence)\nstderr: ", want goroutine dump

Root cause. 27cbe61 (test(lsp): fail fast with the server's goroutine dump when it stays silent) made readBy send SIGQUIT to a silent server and quote its stderr in the failure. The test harness captures stderr through cmd.Stderr = &lockedBuffer{}. Because that writer is not an *os.File, os/exec reads the child's stderr pipe on a goroutine of its own and only guarantees the copy is complete when cmd.Wait returns. readBy read s.stderr.String() as soon as the child's stdout hit EOF — the child had already exited with its dump written to the pipe, but nothing had waited for the copy to land in the buffer. Whether the dump appeared in the failure message was a race against that goroutine, lost under load (the race detector, a busy runner).

Fix. The two killed-server failure paths in readBy now go through killedStderr, which waits for the process (waitStatus, bounded by the same 10 s the 5 s Kill fallback in dumpAndKill makes safe) before reading the buffer:

func (s *session) killedStderr() string {
	s.waitStatus(10 * time.Second)
	return s.stderr.String()
}

waitStatus records that the process was waited for, so the t.Cleanup does not wait twice. The non-killed paths are unchanged: there the server is still running and its stderr is informational.

How it was verified

Reproduced on develop and confirmed gone with the fix, same machine and load (16 busy-loop processes, race-instrumented test binary, -test.count=100):

binary GOMAXPROCS=1 GOMAXPROCS=2
develop (65b98e5) 7 / 100 fail want goroutine dump 1 / 100
this branch 0 / 100 0 / 100

Gates: gofmt -l . prints nothing; go build ./...; go vet ./...; go test -count=1 -race ./cmd/sysml-lsp; go test -count=1 ./... with OPENSYSML_REQUIRE_TRAINING_CORPUS=1 OPENSYSML_REQUIRE_PILOT_CORPORA=1 OPENSYSML_REQUIRE_PILOT_LIBRARY_XMI=1; python3 scripts/changelog.py check.

Checklist

  • make test and make lint pass locally
  • Tests added or updated for the change
  • Documentation extended where it already covers the surface (test harness only; no documented surface changes)
  • Changelog entry added as changes/unreleased/<slug>.<section>.md, not as an edit to CHANGELOG.md
  • baselines regenerated and make docs-counts run if a gate count moved (none moved)
  • No internal work-item labels (waves, slices, F4, K5) in the body, docs, or changelog

Co-Authored-By: jason.han <hanhuijun@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration
devin-ai-integration Bot marked this pull request as ready for review September 26, 2026 02:11

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

This branch has not been deployed

No deployments
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