Skip to content

fix(intra): don't discard rwext read/write deadlines when TCP_USER_TIMEOUT is set #249

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
varunagarwal-pro wants to merge 1 commit into from
Closed

fix(intra): don't discard rwext read/write deadlines when TCP_USER_TIMEOUT is set #249

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions intra/common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,12 +312,20 @@ func (h *baseHandler) forward(local, remote net.Conn, smm *FlowSummary) {
isrwext := false
didSet := false
timeoutsecs := 0
// enable core.Pipe (sendfile/zero-copy) optimizations on TCP if
// read & write deadlines are not set (as in rwext is effectively
// a no-op) by unwrapping the underlying remote conn from rwext.
// enable core.Pipe (sendfile/zero-copy) optimizations on TCP only
// when no read/write deadline is actually configured (timeoutsecs
// <= 0), in which case rwext is a no-op wrapper and unwrapping is
// safe. Do NOT unwrap merely because didSet is true: SetTimeout
// (via core.SetTimeoutSockOpt) only sets TCP_USER_TIMEOUT, which
// bounds unacknowledged *writes*, not idle *reads*. If remote is
// unwrapped here while a positive timeoutsecs is configured, the
// only mechanism that can bound a stalled Read() (rwext's
// extendr/extendw, which set a real per-call deadline) is lost,
// and a peer that silently stops sending (no RST/FIN) causes
// Read() -- and thus this whole forward() -- to block forever.
if r, ok := remote.(rwext); ok {
isrwext = true
if timeoutsecs, didSet = r.SetTimeout(); didSet || timeoutsecs <= 0 {
if timeoutsecs, didSet = r.SetTimeout(); timeoutsecs <= 0 {

@github-actions github-actions Bot Sep 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[bug · medium]
Dropping the didSet clause means remote now stays wrapped in rwext for every positive-timeout case (previously TCP with a successfully-set sockopt was unwrapped to *net.TCPConn). This has a side effect beyond the intended deadline fix: rwext embeds net.Conn but does not implement core.DuplexCloser/core.TCPConn (no CloseRead/CloseWrite; see the TODO? var _ core.DuplexCloser = (*rwext)(nil) in rwconn.go). Consequently core.CloseOp(remote, core.CopW/CopR) in upload/download falls through to the io.Closer case and performs a full Close() instead of a TCP half-close, which can abort the opposite direction of the relay (e.g. a client that half-closes after its request body while the server is still sending the response). Suggest making rwext implement CloseRead/CloseWrite (delegating to the underlying conn when it supports it) so per-call deadline extension and graceful half-close can coexist.

remote = r.Unwrap() // c may be *net.TCPConn or *demuxconn or *dialers.retrier|splitter
}
}
Expand Down
Loading