Skip to content

Commit

Permalink
wgengine/magicsock: make receive from didCopy respect cancellation.
Browse files Browse the repository at this point in the history
Very rarely, cancellation occurs between a successful send on derpRecvCh
and a call to copyBuf on the receiving side.
Without this patch, this situation results in <-copyBuf blocking indefinitely.

Signed-off-by: Dmytro Shynkevych <dmytro@tailscale.com>
  • Loading branch information
Dmytro Shynkevych committed Jul 16, 2020
1 parent 1f92312 commit 8918985
Showing 1 changed file with 9 additions and 1 deletion.
10 changes: 9 additions & 1 deletion wgengine/magicsock/magicsock.go
Original file line number Diff line number Diff line change
Expand Up @@ -1160,7 +1160,15 @@ func (c *Conn) runDerpReader(ctx context.Context, derpFakeAddr netaddr.IPPort, d
case <-ctx.Done():
return
case c.derpRecvCh <- res:
<-didCopy
continue

This comment has been minimized.

Copy link
@bradfitz

bradfitz Jul 16, 2020

Member

I don't think this is correct. Did you mean break? But that's automatic.

This means the code below never runs, which means they're no synchronization on the derp buffer that <-didCopy was waiting for.

The didCopy is never used now.

This comment has been minimized.

Copy link
@bradfitz

bradfitz Jul 16, 2020

Member

I think you meant this:
#563

}
// The copy will not happen if connCtx is cancelled before we reach copyBuf.
// This has resulted in a rare inifite wait in practice.
select {
case <-ctx.Done():
return
case <-didCopy:
continue
}
}
}
Expand Down

0 comments on commit 8918985

Please sign in to comment.