Skip to content

Commit 7407a66

Browse files
committed
[WIP]: fix config data race
This fixes the data-race in the config. Depends on: ipfs/go-ipfs-config#16. This does not fix #4942 as there's still a logical race: parallel config updates clobber each other. However, we'll still need the Clone function for the new `--dry-run` flag introduced in #5455. License: MIT Signed-off-by: Steven Allen <[email protected]>
1 parent e92ace1 commit 7407a66

File tree

2 files changed

+13
-3
lines changed

2 files changed

+13
-3
lines changed

core/commands/config.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,11 @@ func transformConfig(configRoot string, configName string, transformer config.Tr
379379
return err
380380
}
381381

382+
cfg, err = cfg.Clone()
383+
if err != nil {
384+
return err
385+
}
386+
382387
err = transformer(cfg)
383388
if err != nil {
384389
return err

repo/fsrepo/fsrepo.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -476,9 +476,11 @@ func (r *FSRepo) Close() error {
476476
return r.lockfile.Close()
477477
}
478478

479+
// Config the current config. This function DOES NOT copy the config. The caller
480+
// MUST NOT modify it without first calling `Clone`.
481+
//
479482
// Result when not Open is undefined. The method may panic if it pleases.
480483
func (r *FSRepo) Config() (*config.Config, error) {
481-
482484
// It is not necessary to hold the package lock since the repo is in an
483485
// opened state. The package lock is _not_ meant to ensure that the repo is
484486
// 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 {
546548
if err := serialize.WriteConfigFile(configFilename, mapconf); err != nil {
547549
return err
548550
}
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
550554
return nil
551555
}
552556

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.
554559
func (r *FSRepo) SetConfig(updated *config.Config) error {
555560

556561
// packageLock is held to provide thread-safety.

0 commit comments

Comments
 (0)