Skip to content

Commit

Permalink
Only report closure errors for the last connection
Browse files Browse the repository at this point in the history
Safari and iOS Chrome (WebKit) sometimes open multiple TCP
connections that end up being part of the same tcpPacketConn
object in TCPMuxDefault. After some time, the extra connection
is closed even though the main one is still operational. In this
case, the entire peer connection gets disconnected. Instead, we
probably only want to close the entire connection only once there
is no other open TCP connection (this PR).

To be honest, I am not sure why Safari and iOS Chrome do this.
However, there is probably no chance fixing it there.
  • Loading branch information
osery authored and davidzhao committed Sep 8, 2023
1 parent 9590b79 commit dbdc6bd
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 3 deletions.
1 change: 1 addition & 0 deletions AUTHORS.txt
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ Michael MacDonald <mike.macdonald@savantsystems.com>
Mikhail Bragin <misha@wiretrustee.com>
Miroslav Šedivý <sedivy.miro@gmail.com>
Nevio Vesic <nevio@subspace.com>
Ondrej Sery <sery.ondra@gmail.com>
Ori Bernstein <ori@eigenstate.org>
Raja Subramanian <raja.gobi@tutanota.com>
Rasmus Hanning <rasmus@hanning.se>
Expand Down
10 changes: 7 additions & 3 deletions tcp_packet_conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,11 @@ func (t *tcpPacketConn) startReading(conn net.Conn) {
n, err := readStreamingPacket(conn, buf)
if err != nil {
t.params.Logger.Warnf("Failed to read streaming packet: %s", err)
t.handleRecv(streamingPacket{nil, conn.RemoteAddr(), err})
t.removeConn(conn)
last := t.removeConn(conn)
// Only propagate connection closure errors if no other open connection exists.
if last || !(errors.Is(err, io.EOF) || errors.Is(err, net.ErrClosed)) {
t.handleRecv(streamingPacket{nil, conn.RemoteAddr(), err})
}
return
}

Expand Down Expand Up @@ -245,13 +248,14 @@ func (t *tcpPacketConn) closeAndLogError(closer io.Closer) {
}
}

func (t *tcpPacketConn) removeConn(conn net.Conn) {
func (t *tcpPacketConn) removeConn(conn net.Conn) bool {
t.mu.Lock()
defer t.mu.Unlock()

t.closeAndLogError(conn)

delete(t.conns, conn.RemoteAddr().String())
return len(t.conns) == 0
}

func (t *tcpPacketConn) Close() error {
Expand Down

0 comments on commit dbdc6bd

Please sign in to comment.