From 0181a4d0ac46152917476e58c9df8d25c14cbb54 Mon Sep 17 00:00:00 2001 From: Avery Pennarun Date: Fri, 30 Apr 2021 06:08:26 -0400 Subject: [PATCH] ipnlocal: don't pause the controlclient until we get at least one netmap. Without this, macOS would fail to display its menu state correctly if you started it while !WantRunning. It relies on the netmap in order to show the logged-in username. Signed-off-by: Avery Pennarun --- control/controlclient/auto.go | 3 ++- ipn/ipnlocal/local.go | 5 +++-- ipn/ipnlocal/state_test.go | 6 +++++- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/control/controlclient/auto.go b/control/controlclient/auto.go index 077b6ad38..fd1823fa9 100644 --- a/control/controlclient/auto.go +++ b/control/controlclient/auto.go @@ -129,6 +129,7 @@ func (c *Auto) SetPaused(paused bool) { if paused == c.paused { return } + c.logf("setPaused(%v)", paused) c.paused = paused if paused { // Only cancel the map routine. (The auth routine isn't expensive @@ -267,7 +268,7 @@ func (c *Auto) authRoutine() { if goal != nil { c.logf("authRoutine: %s; wantLoggedIn=%v", c.state, goal.wantLoggedIn) } else { - c.logf("authRoutine: %s; goal=nil", c.state) + c.logf("authRoutine: %s; goal=nil paused=%v", c.state, c.paused) } c.mu.Unlock() diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 9f88a3f37..db983b1ce 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -234,7 +234,7 @@ func (b *LocalBackend) linkChange(major bool, ifst *interfaces.State) { networkUp := ifst.AnyInterfaceUp() if b.cc != nil { - go b.cc.SetPaused(b.state == ipn.Stopped || !networkUp) + go b.cc.SetPaused((b.state == ipn.Stopped && b.netMap != nil) || !networkUp) } // If the PAC-ness of the network changed, reconfig wireguard+route to @@ -2067,6 +2067,7 @@ func (b *LocalBackend) enterState(newState ipn.State) { b.state = newState prefs := b.prefs cc := b.cc + netMap := b.netMap networkUp := b.prevIfState.AnyInterfaceUp() activeLogin := b.activeLogin authURL := b.authURL @@ -2088,7 +2089,7 @@ func (b *LocalBackend) enterState(newState ipn.State) { b.send(ipn.Notify{State: &newState}) if cc != nil { - cc.SetPaused(newState == ipn.Stopped || !networkUp) + cc.SetPaused((newState == ipn.Stopped && netMap != nil) || !networkUp) } switch newState { diff --git a/ipn/ipnlocal/state_test.go b/ipn/ipnlocal/state_test.go index cb2015c16..7bb3a88b6 100644 --- a/ipn/ipnlocal/state_test.go +++ b/ipn/ipnlocal/state_test.go @@ -751,7 +751,11 @@ func TestStateMachine(t *testing.T) { // NOTE: cc.Shutdown() is correct here, since we didn't call // b.Shutdown() explicitly ourselves. // BUG: UpdateEndpoints should be called here since we're not WantRunning. - assert.Equal([]string{"Shutdown", "New", "UpdateEndpoints", "Login", "pause"}, cc.getCalls()) + // Note: unpause happens because ipn needs to get at least one netmap + // on startup, otherwise UIs can't show the node list, login + // name, etc when in state ipn.Stopped. + // Arguably they shouldn't try. But they currently do. + assert.Equal([]string{"Shutdown", "New", "UpdateEndpoints", "Login", "unpause"}, cc.getCalls()) nn := notifies.drain(2) assert.Equal([]string{}, cc.getCalls())