Avoid double assign of a.network

mafredri · mafredri · commit 6f9b3192399d · 2022-12-02T13:28:34.000Z
diff --git a/agent/agent.go b/agent/agent.go
@@ -210,13 +210,10 @@ func (a *agent) run(ctx context.Context) error {
 	a.closeMutex.Unlock()
 	if network == nil {
 		a.logger.Debug(ctx, "creating tailnet")
-		network, err = a.createTailnet(ctx, metadata.DERPMap)
+		err = a.createTailnet(ctx, metadata.DERPMap)
 		if err != nil {
 			return xerrors.Errorf("create tailnet: %w", err)
 		}
-		a.closeMutex.Lock()
-		a.network = network
-		a.closeMutex.Unlock()
 	} else {
 		// Update the DERP map!
 		network.SetDERPMap(metadata.DERPMap)
@@ -245,9 +242,10 @@ func (a *agent) trackConnGoroutine(fn func()) error {
 	return nil
 }
 
-func (a *agent) createTailnet(ctx context.Context, derpMap *tailcfg.DERPMap) (network *tailnet.Conn, err error) {
+func (a *agent) createTailnet(ctx context.Context, derpMap *tailcfg.DERPMap) (err error) {
 	a.closeMutex.Lock()
 	if a.isClosed() {
+		a.network = nil
 		a.closeMutex.Unlock()
 		return nil, xerrors.New("closed")
 	}
@@ -264,6 +262,9 @@ func (a *agent) createTailnet(ctx context.Context, derpMap *tailcfg.DERPMap) (ne
 	defer func() {
 		if err != nil {
 			network.Close()
+			a.closeMutex.Lock()
+			a.network = nil
+			a.closeMutex.Unlock()
 		}
 	}()
 	a.network = network
@@ -287,7 +288,7 @@ func (a *agent) createTailnet(ctx context.Context, derpMap *tailcfg.DERPMap) (ne
 			go a.sshServer.HandleConn(conn)
 		}
 	}); err != nil {
-		return nil, err
+		return err
 	}
 
 	reconnectingPTYListener, err := network.Listen("tcp", ":"+strconv.Itoa(codersdk.TailnetReconnectingPTYPort))
@@ -327,7 +328,7 @@ func (a *agent) createTailnet(ctx context.Context, derpMap *tailcfg.DERPMap) (ne
 			go a.handleReconnectingPTY(ctx, msg, conn)
 		}
 	}); err != nil {
-		return nil, err
+		return err
 	}
 
 	speedtestListener, err := network.Listen("tcp", ":"+strconv.Itoa(codersdk.TailnetSpeedtestPort))
@@ -355,7 +356,7 @@ func (a *agent) createTailnet(ctx context.Context, derpMap *tailcfg.DERPMap) (ne
 			}
 		}
 	}); err != nil {
-		return nil, err
+		return err
 	}
 
 	statisticsListener, err := network.Listen("tcp", ":"+strconv.Itoa(codersdk.TailnetStatisticsPort))
@@ -386,7 +387,7 @@ func (a *agent) createTailnet(ctx context.Context, derpMap *tailcfg.DERPMap) (ne
 			a.logger.Critical(ctx, "serve statistics HTTP server", slog.Error(err))
 		}
 	}); err != nil {
-		return nil, err
+		return err
 	}
 
 	return network, nil

Original file line number	Diff line number	Diff line change
`@@ -210,13 +210,10 @@ func (a *agent) run(ctx context.Context) error {`
`210`	`210`	`a.closeMutex.Unlock()`
`211`	`211`	`if network == nil {`
`212`	`212`	`a.logger.Debug(ctx, "creating tailnet")`
`213`		`- network, err = a.createTailnet(ctx, metadata.DERPMap)`
	`213`	`+ err = a.createTailnet(ctx, metadata.DERPMap)`
`214`	`214`	`if err != nil {`
`215`	`215`	`return xerrors.Errorf("create tailnet: %w", err)`
`216`	`216`	`}`
`217`		`- a.closeMutex.Lock()`
`218`		`- a.network = network`
`219`		`- a.closeMutex.Unlock()`
`220`	`217`	`} else {`
`221`	`218`	`// Update the DERP map!`
`222`	`219`	`network.SetDERPMap(metadata.DERPMap)`
`@@ -245,9 +242,10 @@ func (a *agent) trackConnGoroutine(fn func()) error {`
`245`	`242`	`return nil`
`246`	`243`	`}`
`247`	`244`
`248`		`-func (a agent) createTailnet(ctx context.Context, derpMap tailcfg.DERPMap) (network *tailnet.Conn, err error) {`
	`245`	`+func (a agent) createTailnet(ctx context.Context, derpMap tailcfg.DERPMap) (err error) {`
`249`	`246`	`a.closeMutex.Lock()`
`250`	`247`	`if a.isClosed() {`
	`248`	`+ a.network = nil`
`251`	`249`	`a.closeMutex.Unlock()`
`252`	`250`	`return nil, xerrors.New("closed")`
`253`	`251`	`}`
`@@ -264,6 +262,9 @@ func (a agent) createTailnet(ctx context.Context, derpMap tailcfg.DERPMap) (ne`
`264`	`262`	`defer func() {`
`265`	`263`	`if err != nil {`
`266`	`264`	`network.Close()`
	`265`	`+ a.closeMutex.Lock()`
	`266`	`+ a.network = nil`
	`267`	`+ a.closeMutex.Unlock()`
`267`	`268`	`}`
`268`	`269`	`}()`
`269`	`270`	`a.network = network`
`@@ -287,7 +288,7 @@ func (a agent) createTailnet(ctx context.Context, derpMap tailcfg.DERPMap) (ne`
`287`	`288`	`go a.sshServer.HandleConn(conn)`
`288`	`289`	`}`
`289`	`290`	`}); err != nil {`
`290`		`- return nil, err`
	`291`	`+ return err`
`291`	`292`	`}`
`292`	`293`
`293`	`294`	`reconnectingPTYListener, err := network.Listen("tcp", ":"+strconv.Itoa(codersdk.TailnetReconnectingPTYPort))`
`@@ -327,7 +328,7 @@ func (a agent) createTailnet(ctx context.Context, derpMap tailcfg.DERPMap) (ne`
`327`	`328`	`go a.handleReconnectingPTY(ctx, msg, conn)`
`328`	`329`	`}`
`329`	`330`	`}); err != nil {`
`330`		`- return nil, err`
	`331`	`+ return err`
`331`	`332`	`}`
`332`	`333`
`333`	`334`	`speedtestListener, err := network.Listen("tcp", ":"+strconv.Itoa(codersdk.TailnetSpeedtestPort))`
`@@ -355,7 +356,7 @@ func (a agent) createTailnet(ctx context.Context, derpMap tailcfg.DERPMap) (ne`
`355`	`356`	`}`
`356`	`357`	`}`
`357`	`358`	`}); err != nil {`
`358`		`- return nil, err`
	`359`	`+ return err`
`359`	`360`	`}`
`360`	`361`
`361`	`362`	`statisticsListener, err := network.Listen("tcp", ":"+strconv.Itoa(codersdk.TailnetStatisticsPort))`
`@@ -386,7 +387,7 @@ func (a agent) createTailnet(ctx context.Context, derpMap tailcfg.DERPMap) (ne`
`386`	`387`	`a.logger.Critical(ctx, "serve statistics HTTP server", slog.Error(err))`
`387`	`388`	`}`
`388`	`389`	`}); err != nil {`
`389`		`- return nil, err`
	`390`	`+ return err`
`390`	`391`	`}`
`391`	`392`
`392`	`393`	`return network, nil`