Skip to content

Commit 5c0d63d

Browse files
authored
fix: Only hold tailnet.*Conn.Close() for a short duration (#4015)
* fix: Only hold `tailnet.*Conn.Close()` for a short duration The long duration could be cause to a test deadlock. * Add closed chan to listener struct
1 parent d4f0a6f commit 5c0d63d

File tree

3 files changed

+35
-14
lines changed

3 files changed

+35
-14
lines changed

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ replace github.com/tcnksm/go-httpstat => github.com/kylecarbs/go-httpstat v0.0.0
4949

5050
// There are a few minor changes we make to Tailscale that we're slowly upstreaming. Compare here:
5151
// https://github.com/tailscale/tailscale/compare/main...coder:tailscale:main
52-
replace tailscale.com => github.com/coder/tailscale v1.1.1-0.20220907193453-fb5ba5ab658d
52+
replace tailscale.com => github.com/coder/tailscale v1.1.1-0.20220912172152-aa5fb7fc6cac
5353

5454
// Switch to our fork that imports fixes from http://github.com/tailscale/ssh.
5555
// See: https://github.com/coder/coder/issues/3371

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -355,8 +355,8 @@ github.com/coder/retry v1.3.0 h1:5lAAwt/2Cm6lVmnfBY7sOMXcBOwcwJhmV5QGSELIVWY=
355355
github.com/coder/retry v1.3.0/go.mod h1:tXuRgZgWjUnU5LZPT4lJh4ew2elUhexhlnXzrJWdyFY=
356356
github.com/coder/ssh v0.0.0-20220811105153-fcea99919338 h1:tN5GKFT68YLVzJoA8AHuiMNJ0qlhoD3pGN3JY9gxSko=
357357
github.com/coder/ssh v0.0.0-20220811105153-fcea99919338/go.mod h1:ZSS+CUoKHDrqVakTfTWUlKSr9MtMFkC4UvtQKD7O914=
358-
github.com/coder/tailscale v1.1.1-0.20220907193453-fb5ba5ab658d h1:IQ8wJn8MfDS+sesYPpn3EDAyvoGMxFvyyE9uWtcfU6w=
359-
github.com/coder/tailscale v1.1.1-0.20220907193453-fb5ba5ab658d/go.mod h1:MO+tWkQp2YIF3KBnnej/mQvgYccRS5Xk/IrEpZ4Z3BU=
358+
github.com/coder/tailscale v1.1.1-0.20220912172152-aa5fb7fc6cac h1:8v0giAuGwJKFXO3J3ZULhjUku7dSLnBSvTQwkBjhSYs=
359+
github.com/coder/tailscale v1.1.1-0.20220912172152-aa5fb7fc6cac/go.mod h1:MO+tWkQp2YIF3KBnnej/mQvgYccRS5Xk/IrEpZ4Z3BU=
360360
github.com/coder/wireguard-go/tun/netstack v0.0.0-20220823170024-a78136eb0cab h1:9yEvRWXXfyKzXu8AqywCi+tFZAoqCy4wVcsXwuvZNMc=
361361
github.com/coder/wireguard-go/tun/netstack v0.0.0-20220823170024-a78136eb0cab/go.mod h1:TCJ66NtXh3urJotTdoYQOHHkyE899vOQl5TuF+WLSes=
362362
github.com/containerd/aufs v0.0.0-20200908144142-dab0cbea06f4/go.mod h1:nukgQABAEopAHvB6j7cnP5zJ+/3aVcE7hCYqvIwAHyE=

tailnet/conn.go

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -387,16 +387,17 @@ func (c *Conn) Closed() <-chan struct{} {
387387
// Close shuts down the Wireguard connection.
388388
func (c *Conn) Close() error {
389389
c.mutex.Lock()
390-
defer c.mutex.Unlock()
391390
select {
392391
case <-c.closed:
392+
c.mutex.Unlock()
393393
return nil
394394
default:
395395
}
396+
close(c.closed)
396397
for _, l := range c.listeners {
397398
_ = l.closeNoLock()
398399
}
399-
close(c.closed)
400+
c.mutex.Unlock()
400401
_ = c.dialer.Close()
401402
_ = c.magicConn.Close()
402403
_ = c.netStack.Close()
@@ -406,6 +407,15 @@ func (c *Conn) Close() error {
406407
return nil
407408
}
408409

410+
func (c *Conn) isClosed() bool {
411+
select {
412+
case <-c.closed:
413+
return true
414+
default:
415+
return false
416+
}
417+
}
418+
409419
// This and below is taken _mostly_ verbatim from Tailscale:
410420
// https://github.com/tailscale/tailscale/blob/c88bd53b1b7b2fcf7ba302f2e53dd1ce8c32dad4/tsnet/tsnet.go#L459-L494
411421

@@ -422,9 +432,14 @@ func (c *Conn) Listen(network, addr string) (net.Listener, error) {
422432
key: lk,
423433
addr: addr,
424434

425-
conn: make(chan net.Conn),
435+
closed: make(chan struct{}),
436+
conn: make(chan net.Conn),
426437
}
427438
c.mutex.Lock()
439+
if c.isClosed() {
440+
c.mutex.Unlock()
441+
return nil, xerrors.New("closed")
442+
}
428443
if c.listeners == nil {
429444
c.listeners = map[listenKey]*listener{}
430445
}
@@ -460,9 +475,12 @@ func (c *Conn) forwardTCP(conn net.Conn, port uint16) {
460475
defer t.Stop()
461476
select {
462477
case ln.conn <- conn:
478+
return
479+
case <-ln.closed:
480+
case <-c.closed:
463481
case <-t.C:
464-
_ = conn.Close()
465482
}
483+
_ = conn.Close()
466484
}
467485

468486
func (c *Conn) forwardTCPToLocal(conn net.Conn, port uint16) {
@@ -506,15 +524,18 @@ type listenKey struct {
506524
}
507525

508526
type listener struct {
509-
s *Conn
510-
key listenKey
511-
addr string
512-
conn chan net.Conn
527+
s *Conn
528+
key listenKey
529+
addr string
530+
conn chan net.Conn
531+
closed chan struct{}
513532
}
514533

515534
func (ln *listener) Accept() (net.Conn, error) {
516-
c, ok := <-ln.conn
517-
if !ok {
535+
var c net.Conn
536+
select {
537+
case c = <-ln.conn:
538+
case <-ln.closed:
518539
return nil, xerrors.Errorf("wgnet: %w", net.ErrClosed)
519540
}
520541
return c, nil
@@ -530,7 +551,7 @@ func (ln *listener) Close() error {
530551
func (ln *listener) closeNoLock() error {
531552
if v, ok := ln.s.listeners[ln.key]; ok && v == ln {
532553
delete(ln.s.listeners, ln.key)
533-
close(ln.conn)
554+
close(ln.closed)
534555
}
535556
return nil
536557
}

0 commit comments

Comments
 (0)