Skip to content

fix: don't fail reads when the close_notify reply cannot be sent - #73

Merged
djc merged 1 commit into
rustls:mainfrom
ArniDagur:fix-close-notify-reply-eagain
Jul 29, 2026
Merged

fix: don't fail reads when the close_notify reply cannot be sent#73
djc merged 1 commit into
rustls:mainfrom
ArniDagur:fix-close-notify-reply-eagain

Conversation

@ArniDagur

@ArniDagur ArniDagur commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

This bug was discovered while creating a reproducer for #72

If the peer stops reading our data and then closes the connection, a read on our side returns an error instead of EOF. Namely:

  • On receiving the peer's close_notify (or a fatal alert), we reply with a close_notify of our own.
  • Our send buffer is full, so the sendmsg fails with EAGAIN, and the read returns Err(WouldBlock).

The reply is best-effort: the session is over whether or not we manage to send it, and the next read would return EOF anyway (read_closed is already set). Log the failure and return EOF.

If the peer stops reading our data and then closes the connection, a
read on our side returns an error instead of EOF. Namely:

- On receiving the peer's close_notify (or a fatal alert), we reply
  with a close_notify of our own, directly via sendmsg.
- Our send buffer is full, so the sendmsg fails with EAGAIN, and the
  read returns Err(WouldBlock).

The reply is best-effort: the session is over whether or not we manage
to send it, and the next read would return EOF anyway (read_closed is
already set). Log the failure and return EOF.
@ArniDagur

Copy link
Copy Markdown
Contributor Author

@djc Any chance you could look at this?

@djc djc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks reasonable to me. @ctz and/or @cpu?

@djc
djc added this pull request to the merge queue Jul 29, 2026
Merged via the queue into rustls:main with commit b004632 Jul 29, 2026
2 checks passed
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.

3 participants