echoserver: wait to write in the SFTP loop - #1250
Merged
Merged
Conversation
A send the socket refused leaves WS_WANT_WRITE, and the retry branch continues past the only tcp_select() in the iteration, which watches reads. Wait on write readiness there, so a peer that has stopped reading costs a descriptor wait rather than a spin. - a buffered send with a willing socket still goes straight around - an error-ready or failed descriptor ends the loop - an interrupted select() retries instead of ending the session
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Improves the SFTP worker loop in echoserver to avoid CPU spinning when the peer stops reading by waiting on write readiness, and updates SCP read helper documentation to match current behavior.
Changes:
- Add write-readiness waiting (
tcp_select_write) whenWS_WANT_WRITEoccurs in the SFTP loop. - Treat interrupted
select()as retryable instead of tearing down the session. - Update
ScpStreamRead()comment to reflect retry behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/wolfscp.c | Updates ScpStreamRead() comment to reflect retry/pass-through behavior. |
| examples/echoserver/echoserver.c | Adds EINTR mapping and waits for write readiness in the SFTP loop to prevent spinning. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The helper drains a WS_EXTDATA and retries the read rather than handing it back, so it is no longer error-code transparent for that one status. Say so where the claim is made, and condense the rest.
ejohnstown
force-pushed
the
sftp-worker-write-wait
branch
from
September 14, 2026 22:23
02d93f5 to
aa92f50
Compare
padelsbach
approved these changes
Sep 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The SFTP retry branch continued past the iteration's only tcp_select(), which watches reads, so a peer that had stopped reading cost a spin rather than a wait. It now waits on write readiness instead.