fix config data race
This fixes a data-race in the config. This does not fix https://github.com/ipfs/go-ipfs/issues/4942 as there's still a logical race: parallel config updates clobber each other. License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>
Steven Allen committed
Oct 23, 2018 at 10:03 UTC
56b6277d26bc2c0348a60067a87a37d08cf88ca1
2 files changed
+17
-9
core/commands/config.go
+9
-6
@@ -401,15 +401,18 @@ func transformConfig(configRoot string, configName string, transformer config.Tr
401
}
402
defer r.Close()
403
404
- cfg, err := r.Config()
404
+ oldCfg, err := r.Config()
405
if err != nil {
406
return nil, nil, err
407
}
408
409
// make a copy to avoid updating repo's config unintentionally
410
- oldCfg := *cfg
411
- newCfg := oldCfg
412
- err = transformer(&newCfg)
410
+ newCfg, err := oldCfg.Clone()
411
+ if err != nil {
412
+ return nil, nil, err
413
+ }
414
+
415
+ err = transformer(newCfg)
416
if err != nil {
417
return nil, nil, err
418
}
@@ -420,13 +423,13 @@ func transformConfig(configRoot string, configName string, transformer config.Tr
423
return nil, nil, err
424
}
425
423
- err = r.SetConfig(&newCfg)
426
+ err = r.SetConfig(newCfg)
427
if err != nil {
428
return nil, nil, err
429
}
430
}
431
429
- return &oldCfg, &newCfg, nil
432
+ return oldCfg, newCfg, nil
433
}
434
435
func getConfig(r repo.Repo, key string) (*ConfigField, error) {
repo/fsrepo/fsrepo.go
+8
-3
@@ -476,9 +476,11 @@ func (r *FSRepo) Close() error {
476
return r.lockfile.Close()
477
}
478
479
+// Config the current config. This function DOES NOT copy the config. The caller
480
+// MUST NOT modify it without first calling `Clone`.
481
+//
482
// Result when not Open is undefined. The method may panic if it pleases.
483
func (r *FSRepo) Config() (*config.Config, error) {
481
-
484
// It is not necessary to hold the package lock since the repo is in an
485
// opened state. The package lock is _not_ meant to ensure that the repo is
486
// thread-safe. The package lock is only meant to guard against removal and
@@ -546,11 +548,14 @@ func (r *FSRepo) setConfigUnsynced(updated *config.Config) error {
548
if err := serialize.WriteConfigFile(configFilename, mapconf); err != nil {
549
return err
550
}
549
- *r.config = *updated // copy so caller cannot modify this private config
551
+ // Do not use `*r.config = ...`. This will modify the *shared* config
552
+ // returned by `r.Config`.
553
+ r.config = updated
554
return nil
555
}
556
553
-// SetConfig updates the FSRepo's config.
557
+// SetConfig updates the FSRepo's config. The user must not modify the config
558
+// object after calling this method.
559
func (r *FSRepo) SetConfig(updated *config.Config) error {
560
561
// packageLock is held to provide thread-safety.