Skip to content

Read deadline on ReadAdapter permanently closes the peer's send buffer #991

Description

@plorenz

Summary

ReadAdapter.ReadPayload runs the tx-side teardown on any error, including a plain read-deadline expiry. A read deadline is recoverable (ReadTimeout.Temporary() returns true and the caller may read again), but the teardown tells the peer this half of the circuit is finished, which closes the peer's send buffer for good.

func (self *ReadAdapter) ReadPayload() ([]byte, map[uint8][]byte, error) {
	data, headers, err := self.x.nextTxPayload(self.Done())
	if err != nil {
		self.cleanup()   // -> x.txCleanup() -> sendWriteFailed()
		return nil, nil, err
	}
	return data, headers, nil
}

nextTxPayload returns exactly two errors: io.EOF (terminal) and *ReadTimeout (recoverable). Both take the cleanup path.

Effects

txCleanup() calls sendWriteFailed(), and the peer handles that flag out of band in SendPayload:

if payload.IsFlagWriteFailedSet() {
	self.payloadBuffer.Close()
}

So one read timeout has two compounding consequences:

  1. The peer's send buffer closes permanently. payloadBuffer.Close() cannot be undone, so the inbound direction of the circuit is dead from then on. The peer's closedTxer is not set, so closeIfRxAndTxDone() does not fire and the peer is left half-closed rather than closed: its LinkSendBuffer.run() exits and its tx side waits on an EOF/CircuitEnd that will never come. The circuit is wedged, not torn down.

  2. The real cleanup is then skipped. cleanedUp is a one-shot CAS, so when io.EOF genuinely arrives later, cleanup() no-ops and the half-close signalling never runs at the right time.

Trigger

conn.SetReadDeadline() on a conn using SDK flow control. The path is chunk_reader.go -> readXgressChunk -> ReadAdapter.ReadPayload. A net.Conn read-timeout loop is an ordinary pattern, so any application doing it wedges its circuit on the first idle period.

Versions

Live in the v2 module, where ziti/edge/network/conn_xgress.go wires NewReadAdapter() into SDK conns. In v1.7.0 through v1.9.0 NewReadAdapter has no callers outside the xgress package, so the defect is latent there.

Fix

Skip the tx-side teardown when the error is a *ReadTimeout, keeping it for terminal errors. Excluding only the recoverable error rather than matching io.EOF positively keeps the fail-safe direction if another terminal error type is added later.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions