Skip to content

Drain buffered GDB packets before polling - #369

Merged
jserv merged 3 commits into
sysprog21:mainfrom
Suzu1Dev:fix/gdb-buffered-packets
Sep 5, 2026
Merged

Drain buffered GDB packets before polling#369
jserv merged 3 commits into
sysprog21:mainfrom
Suzu1Dev:fix/gdb-buffered-packets

Conversation

@Suzu1Dev

@Suzu1Dev Suzu1Dev commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

The RSP transport can read several requests at once, but the session polls
the socket before consuming each request. A debugger waiting for replies
without sending more data leaves the buffered requests stalled. Consume
buffered input before polling again.

Reproduce by negotiating QStartNoAckMode, then sending eight framed
qAttached requests in one write. The regression requires all eight exact,
checksum-valid replies without another socket write. The subprocess exit
status also participates in the raw RSP test verdict.

Validated on Apple M3 Pro, macOS 27.0 (26A5425a), SDK 26.5:

  • Focused baseline reproduction: 3/3 timeouts; fixed: 10/10 passes.
  • make elfuse and make test-gdbstub: all 18 tests pass.
  • Split-packet/shutdown probe: baseline and fixed each pass 3/3.
  • make check-format passes.

Summary by cubic

Fixes the GDB stub stalling when a single socket read buffers multiple RSP packets. Previously the session polled for new input before consuming each buffered request, so a client waiting for replies without sending more data could stall indefinitely. The session now drains buffered packets before polling again, with the buffer-exhaustion check shared between the session loop and transport decoder. A deterministic host regression forces two requests into one transport read, requires both exact, checksum-valid replies before any further client input, and runs in the shared and GDB check lanes.

Written for commit 6ad8748. Summary will update on new commits.

Review in cubic

The RSP transport can read several requests at once, but the session
polls the socket before consuming each request. Remaining requests
stall when the client waits for their replies without sending more data.

Consume buffered input before polling. Add a regression that sends eight
requests together and requires every reply without another socket write.
cubic-dev-ai[bot]

This comment was marked as resolved.

Comment thread src/debug/gdbstub.c Outdated
Comment thread tests/test-gdbstub.sh Outdated
Comment thread tests/test-gdbstub.sh

if sys.argv[2] == "batch":
# No further socket writes may wake the stub while replies are pending.
sock.sendall(frame_packet("qAttached") * 8)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nothing forces the stub to coalesce these eight frames into one read(). If they arrive split, the unfixed loop polls, finds data, and answers all eight, so the guard passes against the bug it exists to catch. 112 bytes in one loopback write makes that unlikely, but the test does not say it is relying on that.

@Suzu1Dev
Suzu1Dev marked this pull request as draft September 4, 2026 22:17
The session loop and transport decoder both decide when buffered
input is exhausted. Share the transport predicate so changes to the
buffer representation have one definition to update.
Force two requests into one transport read and require both replies
before any further client input. Exercise the real session with a
nonblocking poll wrapper so an unconditional poll fails on the missing
reply instead of hanging the test. Run the host case from the GDB and
shared check lanes.

Compare reply checksum text against the sender's two-digit lowercase
format, and document the TCP batch test's coalescing assumption.
@Suzu1Dev
Suzu1Dev marked this pull request as ready for review September 5, 2026 00:07
cubic-dev-ai[bot]

This comment was marked as resolved.

@jserv
jserv merged commit b54747a into sysprog21:main Sep 5, 2026
15 checks passed
@jserv

jserv commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Thank @Suzu1Dev for contributing!

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.

2 participants