config: rework saving to be atomic and asynchronous

Config.save used os.WriteFile, which truncates the target before writing: killing awl in that window destroyed the config, losing the node's private key and known peers irrecoverably, and readers in other processes (awl-tray, CLI subcommands) could see the truncated file. A config that failed to parse was then quietly discarded — callers fall back to a fresh default that overwrites the same path with a new identity on the next start.

Making the write atomic turns a save into an fsync, ~7ms instead of ~0.1ms, and it happened inline under the caller's lock. Tunnel's VPN gateway setters save while holding peersLock, which HandleReadPackets needs for every batch read from the TUN, so the packet path stalled for the duration of the write.

- Writes go through moby/sys/atomicwriter: staged next to the target, fsynced, renamed over it.
- An unparseable config is copied to config_awl.json.corrupt-<ts> before the fallback, which is now logged as an error rather than a warning indistinguishable from a first run.
- Save hands the request to a single writer goroutine over a capacity-one channel and returns. The writer marshals at write time, so a burst collapses into one write of the newest state.
- Config.Close flushes, stops the writer and closes the emitter, which was never released before — a leak on every gomobile restart.
- NewConfigReadOnly / LoadConfigReadOnly for configs that are only read (CLI, tray without a running server, gomobile GetConfig and DnsServerIP): no writer, no emitter, nothing to close.
- UpdatePeerFields saves and emits only when something other than LastSeen changed. Status exchanges are the dominant caller and usually change nothing else.

Author: pymq

application.go

@@ -3,6 +3,7 @@ package awl
 import (
 	"context"
 	"embed"
+	"errors"
 	"fmt"
 	"io/fs"
 	"net"
@@ -276,8 +277,16 @@ func (a *Application) SetupLoggerAndConfig(appType config.AppType) *log.ZapEvent
 	a.logger = log.Logger("awl")
 	a.Conf = conf
 
-	if loadConfigErr != nil {
-		a.logger.Warnf("failed to read config file, creating new one: %v", loadConfigErr)
+	if errors.Is(loadConfigErr, fs.ErrNotExist) {
+		// First run: there is simply no config yet.
+		a.logger.Infof("no config file found, creating new one")
+	} else if loadConfigErr != nil {
+		// The file is there but unusable. We are about to run with a new
+		// identity and no known peers, and the first save will overwrite the
+		// old file, so this is data loss and must not read as a routine
+		// warning. LoadConfig has already copied a corrupted config aside.
+		a.logger.Errorf("failed to read existing config file, starting with a new one "+
+			"(previous identity and known peers will not be used): %v", loadConfigErr)
 	}
 	a.logger.Infof("Anywherelan %s (%s %s-%s)", config.Version, runtime.Version(), runtime.GOOS, runtime.GOARCH)
 	a.logger.Infof("Initializing app in %s directory", conf.DataDir())
@@ -329,7 +338,7 @@ func (a *Application) Close() {
 			a.logger.Errorf("closing vpn: %v", err)
 		}
 	}
-	a.Conf.Save()
+	a.Conf.Close()
 }
 
 func (a *Application) makeP2pHostConfig() p2p.HostConfig {

cli/cli.go

@@ -14,7 +14,6 @@ import (
 
 	"github.com/GrigoryKrasnochub/updaterini"
 	"github.com/ipfs/go-log/v2"
-	"github.com/libp2p/go-libp2p/p2p/host/eventbus"
 	"github.com/urfave/cli/v2"
 
 	"github.com/anywherelan/awl/api/apiclient"
@@ -518,7 +517,7 @@ func (a *Application) init() {
 				Action: func(c *cli.Context) error {
 					_, _ = fmt.Fprintf(a.cliapp.Writer, "current version: %s\n", config.Version)
 
-					conf, err := config.LoadConfig(a.appType, eventbus.NewBus())
+					conf, err := config.LoadConfigReadOnly(a.appType)
 					if err != nil {
 						return fmt.Errorf("update: read config: %v", err)
 					}
@@ -570,7 +569,7 @@ func (a *Application) initApiConnection(c *cli.Context) error {
 		return a.initApiFromAddr(apiAddr, username, password)
 	}
 
-	conf, errConfig := config.LoadConfig(a.appType, eventbus.NewBus())
+	conf, errConfig := config.LoadConfigReadOnly(a.appType)
 	if errConfig == nil {
 		if username == "" && password == "" {
 			username = conf.HttpBasicAuth.Username

cmd/awl-tray/go.mod

@@ -97,6 +97,8 @@ require (
 	github.com/mikioh/tcpinfo v0.0.0-20190314235526-30a79bb1804b // indirect
 	github.com/mikioh/tcpopt v0.0.0-20190314235656-172688c1accc // indirect
 	github.com/minio/sha256-simd v1.0.1 // indirect
+	github.com/moby/sys/atomicwriter v0.1.0 // indirect
+	github.com/moby/sys/sequential v0.6.0 // indirect
 	github.com/mr-tron/base58 v1.3.0 // indirect
 	github.com/multiformats/go-base32 v0.1.0 // indirect
 	github.com/multiformats/go-base36 v0.2.0 // indirect

cmd/awl-tray/go.sum

@@ -384,6 +384,10 @@ github.com/mitchellh/iochan v1.0.0/go.mod h1:JwYml1nuB7xOzsp52dPpHFffvOCDupsG0Qu
 github.com/mitchellh/mapstructure v0.0.0-20160808181253-ca63d7c062ee/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y=
 github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y=
 github.com/mitchellh/mapstructure v1.4.1/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo=
+github.com/moby/sys/atomicwriter v0.1.0 h1:kw5D/EqkBwsBFi0ss9v1VG3wIkVhzGvLklJ+w3A14Sw=
+github.com/moby/sys/atomicwriter v0.1.0/go.mod h1:Ul8oqv2ZMNHOceF643P6FKPXeCmYtlQMvpizfsSoaWs=
+github.com/moby/sys/sequential v0.6.0 h1:qrx7XFUd/5DxtqcoH1h438hF5TmOvzC/lspjy7zgvCU=
+github.com/moby/sys/sequential v0.6.0/go.mod h1:uyv8EUTrca5PnDsdMGXhZe6CCe8U/UiTWd+lL+7b/Ko=
 github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
 github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0=
 github.com/modern-go/reflect2 v1.0.1/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0=

cmd/awl-tray/main.go

@@ -15,7 +15,6 @@ import (
 	"github.com/gen2brain/beeep"
 	"github.com/ipfs/go-log/v2"
 	"github.com/libp2p/go-libp2p/core/network"
-	"github.com/libp2p/go-libp2p/p2p/host/eventbus"
 
 	"github.com/anywherelan/awl"
 	"github.com/anywherelan/awl/awldns"
@@ -36,7 +35,7 @@ func getConfig() (*config.Config, error) {
 	if app != nil {
 		return app.Conf, nil
 	}
-	return config.LoadConfig(appType, eventbus.NewBus())
+	return config.LoadConfigReadOnly(appType)
 }
 
 func main() {

cmd/gomobile-lib/main.go

@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"os"
 
-	"github.com/libp2p/go-libp2p/p2p/host/eventbus"
 	"golang.zx2c4.com/wireguard/tun"
 
 	"github.com/anywherelan/awl"
@@ -39,9 +38,9 @@ func GetConfig() string {
 		panic("call to GetConfig before Setup")
 	}
 
-	conf, loadConfigErr := config.LoadConfig(appType, eventbus.NewBus())
+	conf, loadConfigErr := config.LoadConfigReadOnly(appType)
 	if loadConfigErr != nil {
-		conf = config.NewConfig(appType, eventbus.NewBus())
+		conf = config.NewConfigReadOnly(appType)
 	}
 
 	data := conf.Export()
@@ -186,9 +185,9 @@ func DnsServerIP() string {
 		panic("call to DnsServerIP before Setup")
 	}
 
-	conf, loadConfigErr := config.LoadConfig(appType, eventbus.NewBus())
+	conf, loadConfigErr := config.LoadConfigReadOnly(appType)
 	if loadConfigErr != nil {
-		conf = config.NewConfig(appType, eventbus.NewBus())
+		conf = config.NewConfigReadOnly(appType)
 	}
 	ip := conf.NetstackDNSIP()
 	if ip == nil {

config/config.go

@@ -54,6 +54,17 @@ type (
 		// desktop for a future userspace-netstack path.
 		netstackDNSIP net.IP
 
+		// saveTrigger carries save requests to the writer goroutine. Capacity
+		// one, so a request that finds it full is simply dropped: a write is
+		// already queued and marshals at write time, which is what makes a
+		// burst of saves collapse into a single write of the newest state.
+		//
+		// nil on a config from NewConfigReadOnly/LoadConfigReadOnly, which has no writer.
+		saveTrigger chan struct{}
+		closeCh     chan struct{}
+		writerDone  chan struct{}
+		closeOnce   sync.Once
+
 		Version               string                 `json:"version"`
 		LoggerLevel           string                 `json:"loggerLevel"`
 		HttpListenAddress     string                 `json:"httpListenAddress"`
@@ -171,17 +182,22 @@ type (
 	}
 )
 
+// Save schedules a write of the config and returns immediately; the writer
+// goroutine performs it. It is safe to call with or without the config lock
+// held, because it touches no config state — only saveTrigger.
 func (c *Config) Save() {
-	c.RLock()
-	c.save()
-	c.RUnlock()
-}
+	if c.saveTrigger == nil {
+		// A read-only config has no writer. Reaching here means something
+		// mutated a snapshot and expected it to persist.
+		logger.DPanicf("Save called on a read-only config")
+		return
+	}
 
-// SaveLocked persists the config without taking the lock. The caller must
-// already hold c.Lock(). Use this when atomically batching mutations and
-// persistence under a single critical section.
-func (c *Config) SaveLocked() {
-	c.save()
+	select {
+	case c.saveTrigger <- struct{}{}:
+	default:
+		// a write is already queued and will pick up the newest state
+	}
 }
 
 func (c *Config) IsUniqPeerAlias(excludePeerID, alias string) bool {
@@ -251,12 +267,12 @@ func (c *Config) RemovePeer(peerID string) (KnownPeer, bool) {
 	knownPeer, exists := c.KnownPeers[peerID]
 	if exists {
 		delete(c.KnownPeers, peerID)
-		c.save()
+		c.Save()
 	}
 	c.Unlock()
 
 	if exists {
-		_ = c.emitter.Emit(awlevent.KnownPeerChanged{})
+		c.emitKnownPeerChanged()
 	}
 
 	return knownPeer, exists
@@ -265,21 +281,21 @@ func (c *Config) RemovePeer(peerID string) (KnownPeer, bool) {
 func (c *Config) UpsertPeer(peer KnownPeer) {
 	c.Lock()
 	c.KnownPeers[peer.PeerID] = peer
-	c.save()
+	c.Save()
 	c.Unlock()
 
-	_ = c.emitter.Emit(awlevent.KnownPeerChanged{})
+	c.emitKnownPeerChanged()
 }
 
 func (c *Config) UpsertPeerUnlocked(peer KnownPeer) {
 	c.KnownPeers[peer.PeerID] = peer
-	c.save()
+	c.Save()
 
-	_ = c.emitter.Emit(awlevent.KnownPeerChanged{})
+	c.emitKnownPeerChanged()
 }
 
 // UpdatePeerFields atomically applies mutate to the stored KnownPeer under the
-// write lock and persists the change. It returns false if the peer is unknown.
+// write lock. It returns false if the peer is unknown.
 //
 // mutate must only change the fields it owns and must NOT replace the struct
 // wholesale, so that fields updated concurrently by other callers are not
@@ -288,19 +304,32 @@ func (c *Config) UpsertPeerUnlocked(peer KnownPeer) {
 func (c *Config) UpdatePeerFields(peerID string, mutate func(*KnownPeer)) bool {
 	c.Lock()
 	knownPeer, ok := c.KnownPeers[peerID]
+	changed := false
 	if ok {
-		mutate(&knownPeer)
-		c.KnownPeers[peerID] = knownPeer
-		c.save()
+		updated := knownPeer
+		mutate(&updated)
+		c.KnownPeers[peerID] = updated
+
+		changed = peerChangedIgnoringLastSeen(knownPeer, updated)
+		if changed {
+			c.Save()
+		}
 	}
 	c.Unlock()
 
-	if ok {
-		_ = c.emitter.Emit(awlevent.KnownPeerChanged{})
+	if changed {
+		c.emitKnownPeerChanged()
 	}
 	return ok
 }
 
+// peerChangedIgnoringLastSeen reports whether anything except LastSeen differs.
+func peerChangedIgnoringLastSeen(before, after KnownPeer) bool {
+	before.LastSeen = time.Time{}
+	after.LastSeen = time.Time{}
+	return before != after
+}
+
 func (c *Config) UpdatePeerLastSeen(peerID string) {
 	c.Lock()
 	knownPeer, ok := c.KnownPeers[peerID]
@@ -323,7 +352,7 @@ func (c *Config) RemoveBlockedPeer(peerID string) {
 	_, exists := c.BlockedPeers[peerID]
 	if exists {
 		delete(c.BlockedPeers, peerID)
-		c.save()
+		c.Save()
 	}
 	c.Unlock()
 }
@@ -337,7 +366,7 @@ func (c *Config) UpsertBlockedPeer(peerID, displayName string) {
 	blockedPeer.PeerID = peerID
 	blockedPeer.DisplayName = displayName
 	c.BlockedPeers[peerID] = blockedPeer
-	c.save()
+	c.Save()
 	c.Unlock()
 }
 
@@ -348,7 +377,7 @@ func (c *Config) SetIdentity(key crypto.PrivKey, id peer.ID) {
 
 	c.P2pNode.Identity = identity
 	c.P2pNode.PeerID = id.String()
-	c.save()
+	c.Save()
 	c.Unlock()
 }
 
@@ -474,18 +503,75 @@ func (c *Config) Export() []byte {
 	return data
 }
 
-func (c *Config) save() {
+// Close stops the writer goroutine after a final flush and releases the event
+// emitter. It is idempotent and safe on a read-only config, which has neither.
+//
+// Callers of NewConfig and LoadConfig must call it, otherwise the writer
+// goroutine and the emitter's slot on the event bus outlive the config.
+func (c *Config) Close() {
+	c.closeOnce.Do(func() {
+		if c.saveTrigger != nil {
+			close(c.closeCh)
+			<-c.writerDone
+		}
+		if c.emitter != nil {
+			if err := c.emitter.Close(); err != nil {
+				logger.Errorf("Close config emitter: %v", err)
+			}
+		}
+	})
+}
+
+// startWriter launches the goroutine that owns writing this config to disk.
+// Called by the constructors that hand out a config someone will mutate; the
+// read-only ones skip it, which is what leaves saveTrigger nil.
+func (c *Config) startWriter() {
+	c.saveTrigger = make(chan struct{}, 1)
+	c.closeCh = make(chan struct{})
+	c.writerDone = make(chan struct{})
+
+	go c.writer()
+}
+
+func (c *Config) writer() {
+	defer close(c.writerDone)
+
+	for {
+		select {
+		case <-c.saveTrigger:
+			c.writeNow()
+		case <-c.closeCh:
+			// final flush
+			c.writeNow()
+			return
+		}
+	}
+}
+
+// writeNow is only ever called from the writer goroutine.
+func (c *Config) writeNow() {
+	c.RLock()
 	data, err := json.MarshalIndent(c, "", "  ")
+	c.RUnlock()
 	if err != nil {
+		// A marshal failure is a bug in the config structs, not an
+		// environmental problem, so it stays a DPanic.
 		logger.DPanicf("Marshal config: %v", err)
 		return
 	}
-	path := c.path()
-	err = os.WriteFile(path, data, filesPerm)
-	if err != nil {
+
+	if err = writeFileAtomic(c.path(), data); err != nil {
 		logger.DPanicf("Save config: %v", err)
 	}
-	ChownFileIfNeeded(path)
+}
+
+// emitKnownPeerChanged announces a change to the known peers.
+func (c *Config) emitKnownPeerChanged() {
+	// The emitter is nil on a read-only config.
+	if c.emitter == nil {
+		return
+	}
+	_ = c.emitter.Emit(awlevent.KnownPeerChanged{})
 }
 
 func (c *Config) path() string {