From 24fa616e7330229579b89d37254ac050bc7d6069 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Mon, 1 Mar 2021 21:34:41 -0800 Subject: [PATCH] wgengine/monitor: make Darwin monitor shut down cleanly, add test Don't use os.NewFile or (*os.File).Close on the AF_ROUTE socket. It apparently does weird things to the fd and at least doesn't seem to close it. Just use the unix package. The test doesn't actually fail reliably before the fix, though. It was an attempt. But this fixes the integration tests. Signed-off-by: Brad Fitzpatrick --- wgengine/monitor/monitor_darwin.go | 19 ++++++++++++------- wgengine/monitor/monitor_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 7 deletions(-) create mode 100644 wgengine/monitor/monitor_test.go diff --git a/wgengine/monitor/monitor_darwin.go b/wgengine/monitor/monitor_darwin.go index 1d7a106a4..2ae9c4433 100644 --- a/wgengine/monitor/monitor_darwin.go +++ b/wgengine/monitor/monitor_darwin.go @@ -7,7 +7,7 @@ package monitor import ( "fmt" "log" - "os" + "sync" "golang.org/x/net/route" "golang.org/x/sys/unix" @@ -31,22 +31,27 @@ func newOSMon(logf logger.Logf) (osMon, error) { } return &darwinRouteMon{ logf: logf, - f: os.NewFile(uintptr(fd), "AF_ROUTE"), + fd: fd, }, nil } type darwinRouteMon struct { - logf logger.Logf - f *os.File // AF_ROUTE socket - buf [2 << 10]byte + logf logger.Logf + fd int // AF_ROUTE socket + buf [2 << 10]byte + closeOnce sync.Once } func (m *darwinRouteMon) Close() error { - return m.f.Close() + var err error + m.closeOnce.Do(func() { + err = unix.Close(m.fd) + }) + return err } func (m *darwinRouteMon) Receive() (message, error) { - n, err := m.f.Read(m.buf[:]) + n, err := unix.Read(m.fd, m.buf[:]) if err != nil { return nil, err } diff --git a/wgengine/monitor/monitor_test.go b/wgengine/monitor/monitor_test.go new file mode 100644 index 000000000..9c9019eeb --- /dev/null +++ b/wgengine/monitor/monitor_test.go @@ -0,0 +1,30 @@ +// Copyright (c) 2021 Tailscale Inc & AUTHORS All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package monitor + +import ( + "testing" +) + +func TestMonitorStartClose(t *testing.T) { + mon, err := New(t.Logf) + if err != nil { + t.Fatal(err) + } + mon.Start() + if err := mon.Close(); err != nil { + t.Fatal(err) + } +} + +func TestMonitorJustClose(t *testing.T) { + mon, err := New(t.Logf) + if err != nil { + t.Fatal(err) + } + if err := mon.Close(); err != nil { + t.Fatal(err) + } +}