Skip to content

Commit b918a6f

Browse files
authored
Merge pull request #266 from anywherelan/config-save-rework
config: rework saving to be atomic and asynchronous
2 parents 26b8cd0 + 62543fd commit b918a6f

14 files changed

Lines changed: 439 additions & 54 deletions

File tree

application.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package awl
33
import (
44
"context"
55
"embed"
6+
"errors"
67
"fmt"
78
"io/fs"
89
"net"
@@ -276,8 +277,16 @@ func (a *Application) SetupLoggerAndConfig(appType config.AppType) *log.ZapEvent
276277
a.logger = log.Logger("awl")
277278
a.Conf = conf
278279

279-
if loadConfigErr != nil {
280-
a.logger.Warnf("failed to read config file, creating new one: %v", loadConfigErr)
280+
if errors.Is(loadConfigErr, fs.ErrNotExist) {
281+
// First run: there is simply no config yet.
282+
a.logger.Infof("no config file found, creating new one")
283+
} else if loadConfigErr != nil {
284+
// The file is there but unusable. We are about to run with a new
285+
// identity and no known peers, and the first save will overwrite the
286+
// old file, so this is data loss and must not read as a routine
287+
// warning. LoadConfig has already copied a corrupted config aside.
288+
a.logger.Errorf("failed to read existing config file, starting with a new one "+
289+
"(previous identity and known peers will not be used): %v", loadConfigErr)
281290
}
282291
a.logger.Infof("Anywherelan %s (%s %s-%s)", config.Version, runtime.Version(), runtime.GOOS, runtime.GOARCH)
283292
a.logger.Infof("Initializing app in %s directory", conf.DataDir())
@@ -329,7 +338,7 @@ func (a *Application) Close() {
329338
a.logger.Errorf("closing vpn: %v", err)
330339
}
331340
}
332-
a.Conf.Save()
341+
a.Conf.Close()
333342
}
334343

335344
func (a *Application) makeP2pHostConfig() p2p.HostConfig {

cli/cli.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import (
1414

1515
"github.com/GrigoryKrasnochub/updaterini"
1616
"github.com/ipfs/go-log/v2"
17-
"github.com/libp2p/go-libp2p/p2p/host/eventbus"
1817
"github.com/urfave/cli/v2"
1918

2019
"github.com/anywherelan/awl/api/apiclient"
@@ -518,7 +517,7 @@ func (a *Application) init() {
518517
Action: func(c *cli.Context) error {
519518
_, _ = fmt.Fprintf(a.cliapp.Writer, "current version: %s\n", config.Version)
520519

521-
conf, err := config.LoadConfig(a.appType, eventbus.NewBus())
520+
conf, err := config.LoadConfigReadOnly(a.appType)
522521
if err != nil {
523522
return fmt.Errorf("update: read config: %v", err)
524523
}
@@ -570,7 +569,7 @@ func (a *Application) initApiConnection(c *cli.Context) error {
570569
return a.initApiFromAddr(apiAddr, username, password)
571570
}
572571

573-
conf, errConfig := config.LoadConfig(a.appType, eventbus.NewBus())
572+
conf, errConfig := config.LoadConfigReadOnly(a.appType)
574573
if errConfig == nil {
575574
if username == "" && password == "" {
576575
username = conf.HttpBasicAuth.Username

cmd/awl-tray/go.mod

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,8 @@ require (
9797
github.com/mikioh/tcpinfo v0.0.0-20190314235526-30a79bb1804b // indirect
9898
github.com/mikioh/tcpopt v0.0.0-20190314235656-172688c1accc // indirect
9999
github.com/minio/sha256-simd v1.0.1 // indirect
100+
github.com/moby/sys/atomicwriter v0.1.0 // indirect
101+
github.com/moby/sys/sequential v0.6.0 // indirect
100102
github.com/mr-tron/base58 v1.3.0 // indirect
101103
github.com/multiformats/go-base32 v0.1.0 // indirect
102104
github.com/multiformats/go-base36 v0.2.0 // indirect

cmd/awl-tray/go.sum

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,10 @@ github.com/mitchellh/iochan v1.0.0/go.mod h1:JwYml1nuB7xOzsp52dPpHFffvOCDupsG0Qu
384384
github.com/mitchellh/mapstructure v0.0.0-20160808181253-ca63d7c062ee/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y=
385385
github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y=
386386
github.com/mitchellh/mapstructure v1.4.1/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo=
387+
github.com/moby/sys/atomicwriter v0.1.0 h1:kw5D/EqkBwsBFi0ss9v1VG3wIkVhzGvLklJ+w3A14Sw=
388+
github.com/moby/sys/atomicwriter v0.1.0/go.mod h1:Ul8oqv2ZMNHOceF643P6FKPXeCmYtlQMvpizfsSoaWs=
389+
github.com/moby/sys/sequential v0.6.0 h1:qrx7XFUd/5DxtqcoH1h438hF5TmOvzC/lspjy7zgvCU=
390+
github.com/moby/sys/sequential v0.6.0/go.mod h1:uyv8EUTrca5PnDsdMGXhZe6CCe8U/UiTWd+lL+7b/Ko=
387391
github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
388392
github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0=
389393
github.com/modern-go/reflect2 v1.0.1/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0=

cmd/awl-tray/main.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515
"github.com/gen2brain/beeep"
1616
"github.com/ipfs/go-log/v2"
1717
"github.com/libp2p/go-libp2p/core/network"
18-
"github.com/libp2p/go-libp2p/p2p/host/eventbus"
1918

2019
"github.com/anywherelan/awl"
2120
"github.com/anywherelan/awl/awldns"
@@ -36,7 +35,7 @@ func getConfig() (*config.Config, error) {
3635
if app != nil {
3736
return app.Conf, nil
3837
}
39-
return config.LoadConfig(appType, eventbus.NewBus())
38+
return config.LoadConfigReadOnly(appType)
4039
}
4140

4241
func main() {

cmd/gomobile-lib/main.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"fmt"
88
"os"
99

10-
"github.com/libp2p/go-libp2p/p2p/host/eventbus"
1110
"golang.zx2c4.com/wireguard/tun"
1211

1312
"github.com/anywherelan/awl"
@@ -39,9 +38,9 @@ func GetConfig() string {
3938
panic("call to GetConfig before Setup")
4039
}
4140

42-
conf, loadConfigErr := config.LoadConfig(appType, eventbus.NewBus())
41+
conf, loadConfigErr := config.LoadConfigReadOnly(appType)
4342
if loadConfigErr != nil {
44-
conf = config.NewConfig(appType, eventbus.NewBus())
43+
conf = config.NewConfigReadOnly(appType)
4544
}
4645

4746
data := conf.Export()
@@ -186,9 +185,9 @@ func DnsServerIP() string {
186185
panic("call to DnsServerIP before Setup")
187186
}
188187

189-
conf, loadConfigErr := config.LoadConfig(appType, eventbus.NewBus())
188+
conf, loadConfigErr := config.LoadConfigReadOnly(appType)
190189
if loadConfigErr != nil {
191-
conf = config.NewConfig(appType, eventbus.NewBus())
190+
conf = config.NewConfigReadOnly(appType)
192191
}
193192
ip := conf.NetstackDNSIP()
194193
if ip == nil {

config/config.go

Lines changed: 115 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,17 @@ type (
5454
// desktop for a future userspace-netstack path.
5555
netstackDNSIP net.IP
5656

57+
// saveTrigger carries save requests to the writer goroutine. Capacity
58+
// one, so a request that finds it full is simply dropped: a write is
59+
// already queued and marshals at write time, which is what makes a
60+
// burst of saves collapse into a single write of the newest state.
61+
//
62+
// nil on a config from NewConfigReadOnly/LoadConfigReadOnly, which has no writer.
63+
saveTrigger chan struct{}
64+
closeCh chan struct{}
65+
writerDone chan struct{}
66+
closeOnce sync.Once
67+
5768
Version string `json:"version"`
5869
LoggerLevel string `json:"loggerLevel"`
5970
HttpListenAddress string `json:"httpListenAddress"`
@@ -171,17 +182,22 @@ type (
171182
}
172183
)
173184

185+
// Save schedules a write of the config and returns immediately; the writer
186+
// goroutine performs it. It is safe to call with or without the config lock
187+
// held, because it touches no config state — only saveTrigger.
174188
func (c *Config) Save() {
175-
c.RLock()
176-
c.save()
177-
c.RUnlock()
178-
}
189+
if c.saveTrigger == nil {
190+
// A read-only config has no writer. Reaching here means something
191+
// mutated a snapshot and expected it to persist.
192+
logger.DPanicf("Save called on a read-only config")
193+
return
194+
}
179195

180-
// SaveLocked persists the config without taking the lock. The caller must
181-
// already hold c.Lock(). Use this when atomically batching mutations and
182-
// persistence under a single critical section.
183-
func (c *Config) SaveLocked() {
184-
c.save()
196+
select {
197+
case c.saveTrigger <- struct{}{}:
198+
default:
199+
// a write is already queued and will pick up the newest state
200+
}
185201
}
186202

187203
func (c *Config) IsUniqPeerAlias(excludePeerID, alias string) bool {
@@ -251,12 +267,12 @@ func (c *Config) RemovePeer(peerID string) (KnownPeer, bool) {
251267
knownPeer, exists := c.KnownPeers[peerID]
252268
if exists {
253269
delete(c.KnownPeers, peerID)
254-
c.save()
270+
c.Save()
255271
}
256272
c.Unlock()
257273

258274
if exists {
259-
_ = c.emitter.Emit(awlevent.KnownPeerChanged{})
275+
c.emitKnownPeerChanged()
260276
}
261277

262278
return knownPeer, exists
@@ -265,21 +281,21 @@ func (c *Config) RemovePeer(peerID string) (KnownPeer, bool) {
265281
func (c *Config) UpsertPeer(peer KnownPeer) {
266282
c.Lock()
267283
c.KnownPeers[peer.PeerID] = peer
268-
c.save()
284+
c.Save()
269285
c.Unlock()
270286

271-
_ = c.emitter.Emit(awlevent.KnownPeerChanged{})
287+
c.emitKnownPeerChanged()
272288
}
273289

274290
func (c *Config) UpsertPeerUnlocked(peer KnownPeer) {
275291
c.KnownPeers[peer.PeerID] = peer
276-
c.save()
292+
c.Save()
277293

278-
_ = c.emitter.Emit(awlevent.KnownPeerChanged{})
294+
c.emitKnownPeerChanged()
279295
}
280296

281297
// UpdatePeerFields atomically applies mutate to the stored KnownPeer under the
282-
// write lock and persists the change. It returns false if the peer is unknown.
298+
// write lock. It returns false if the peer is unknown.
283299
//
284300
// mutate must only change the fields it owns and must NOT replace the struct
285301
// wholesale, so that fields updated concurrently by other callers are not
@@ -288,19 +304,32 @@ func (c *Config) UpsertPeerUnlocked(peer KnownPeer) {
288304
func (c *Config) UpdatePeerFields(peerID string, mutate func(*KnownPeer)) bool {
289305
c.Lock()
290306
knownPeer, ok := c.KnownPeers[peerID]
307+
changed := false
291308
if ok {
292-
mutate(&knownPeer)
293-
c.KnownPeers[peerID] = knownPeer
294-
c.save()
309+
updated := knownPeer
310+
mutate(&updated)
311+
c.KnownPeers[peerID] = updated
312+
313+
changed = peerChangedIgnoringLastSeen(knownPeer, updated)
314+
if changed {
315+
c.Save()
316+
}
295317
}
296318
c.Unlock()
297319

298-
if ok {
299-
_ = c.emitter.Emit(awlevent.KnownPeerChanged{})
320+
if changed {
321+
c.emitKnownPeerChanged()
300322
}
301323
return ok
302324
}
303325

326+
// peerChangedIgnoringLastSeen reports whether anything except LastSeen differs.
327+
func peerChangedIgnoringLastSeen(before, after KnownPeer) bool {
328+
before.LastSeen = time.Time{}
329+
after.LastSeen = time.Time{}
330+
return before != after
331+
}
332+
304333
func (c *Config) UpdatePeerLastSeen(peerID string) {
305334
c.Lock()
306335
knownPeer, ok := c.KnownPeers[peerID]
@@ -323,7 +352,7 @@ func (c *Config) RemoveBlockedPeer(peerID string) {
323352
_, exists := c.BlockedPeers[peerID]
324353
if exists {
325354
delete(c.BlockedPeers, peerID)
326-
c.save()
355+
c.Save()
327356
}
328357
c.Unlock()
329358
}
@@ -337,7 +366,7 @@ func (c *Config) UpsertBlockedPeer(peerID, displayName string) {
337366
blockedPeer.PeerID = peerID
338367
blockedPeer.DisplayName = displayName
339368
c.BlockedPeers[peerID] = blockedPeer
340-
c.save()
369+
c.Save()
341370
c.Unlock()
342371
}
343372

@@ -348,7 +377,7 @@ func (c *Config) SetIdentity(key crypto.PrivKey, id peer.ID) {
348377

349378
c.P2pNode.Identity = identity
350379
c.P2pNode.PeerID = id.String()
351-
c.save()
380+
c.Save()
352381
c.Unlock()
353382
}
354383

@@ -474,18 +503,75 @@ func (c *Config) Export() []byte {
474503
return data
475504
}
476505

477-
func (c *Config) save() {
506+
// Close stops the writer goroutine after a final flush and releases the event
507+
// emitter. It is idempotent and safe on a read-only config, which has neither.
508+
//
509+
// Callers of NewConfig and LoadConfig must call it, otherwise the writer
510+
// goroutine and the emitter's slot on the event bus outlive the config.
511+
func (c *Config) Close() {
512+
c.closeOnce.Do(func() {
513+
if c.saveTrigger != nil {
514+
close(c.closeCh)
515+
<-c.writerDone
516+
}
517+
if c.emitter != nil {
518+
if err := c.emitter.Close(); err != nil {
519+
logger.Errorf("Close config emitter: %v", err)
520+
}
521+
}
522+
})
523+
}
524+
525+
// startWriter launches the goroutine that owns writing this config to disk.
526+
// Called by the constructors that hand out a config someone will mutate; the
527+
// read-only ones skip it, which is what leaves saveTrigger nil.
528+
func (c *Config) startWriter() {
529+
c.saveTrigger = make(chan struct{}, 1)
530+
c.closeCh = make(chan struct{})
531+
c.writerDone = make(chan struct{})
532+
533+
go c.writer()
534+
}
535+
536+
func (c *Config) writer() {
537+
defer close(c.writerDone)
538+
539+
for {
540+
select {
541+
case <-c.saveTrigger:
542+
c.writeNow()
543+
case <-c.closeCh:
544+
// final flush
545+
c.writeNow()
546+
return
547+
}
548+
}
549+
}
550+
551+
// writeNow is only ever called from the writer goroutine.
552+
func (c *Config) writeNow() {
553+
c.RLock()
478554
data, err := json.MarshalIndent(c, "", " ")
555+
c.RUnlock()
479556
if err != nil {
557+
// A marshal failure is a bug in the config structs, not an
558+
// environmental problem, so it stays a DPanic.
480559
logger.DPanicf("Marshal config: %v", err)
481560
return
482561
}
483-
path := c.path()
484-
err = os.WriteFile(path, data, filesPerm)
485-
if err != nil {
562+
563+
if err = writeFileAtomic(c.path(), data); err != nil {
486564
logger.DPanicf("Save config: %v", err)
487565
}
488-
ChownFileIfNeeded(path)
566+
}
567+
568+
// emitKnownPeerChanged announces a change to the known peers.
569+
func (c *Config) emitKnownPeerChanged() {
570+
// The emitter is nil on a read-only config.
571+
if c.emitter == nil {
572+
return
573+
}
574+
_ = c.emitter.Emit(awlevent.KnownPeerChanged{})
489575
}
490576

491577
func (c *Config) path() string {

0 commit comments

Comments
 (0)