ipn/ipnlocal: fix deadlock in resetControlClientLocked

resetControlClientLocked is called while b.mu was held and
would call cc.Shutdown which would wait for the observer queue
to drain.
However, there may be active callbacks from cc already waiting for
b.mu resulting in a deadlock.

This makes it so that resetControlClientLocked does not call
Shutdown, and instead just returns the value.
It also makes it so that any status received from previous cc
are ignored.

Updates tailscale/corp#12827

Signed-off-by: Maisem Ali <maisem@tailscale.com>
This commit is contained in:
Maisem Ali
2023-09-02 12:04:03 -07:00
committed by Maisem Ali
parent c6fadd6d71
commit d06a75dcd0
6 changed files with 53 additions and 28 deletions

View File

@@ -595,7 +595,7 @@ func (c *Auto) sendStatus(who string, err error, url string, nm *netmap.NetworkM
// Launch a new goroutine to avoid blocking the caller while the observer
// does its thing, which may result in a call back into the client.
c.observerQueue.Add(func() {
c.observer.SetControlClientStatus(new)
c.observer.SetControlClientStatus(c, new)
})
}
@@ -667,6 +667,7 @@ func (c *Auto) Shutdown() {
direct := c.direct
if !closed {
c.closed = true
c.observerQueue.shutdown()
c.cancelAuthCtxLocked()
c.cancelMapCtxLocked()
for _, w := range c.unpauseWaiters {

View File

@@ -25,6 +25,9 @@ const (
// Client represents a client connection to the control server.
// Currently this is done through a pair of polling https requests in
// the Auto client, but that might change eventually.
//
// The Client must be comparable as it is used by the Observer to detect stale
// clients.
type Client interface {
// Shutdown closes this session, which should not be used any further
// afterwards.

View File

@@ -104,7 +104,11 @@ type Direct struct {
// Observer is implemented by users of the control client (such as LocalBackend)
// to get notified of changes in the control client's status.
type Observer interface {
SetControlClientStatus(Status)
// SetControlClientStatus is called when the client has a new status to
// report. The Client is provided to allow the Observer to track which
// Client is reporting the status, allowing it to ignore stale status
// reports from previous Clients.
SetControlClientStatus(Client, Status)
}
type Options struct {