Config is not thread-safe

[Stebalien]

While we try to make it thread-safe by copying the config on read (see Repo.Config()) this doesn't really fix the problem because we have, e.g., maps inside of it. Furthermore, while not safety issue, the profile apply commands don't compose atomically.

Instead of exposing Config(), SetConfig(), BackupConfig(), etc., we should probably provide the following two methods:

ReadConfig(read func (cfg *Config) error) error

// Not sure about the signature but I'd *like* backups to be atomic.
UpdateConfig(update func (cfg *Config) error, backup bool) (backup string, err error)

[qiwaa]

I am very interested in this. Can I help with this?

[Stebalien]

I'd like to get someone (@magik6k? @schomatis?) to comment on the design first.

[magik6k]

Looks good to me

[Stebalien]

@chenminjian then go ahead.

[qiwaa]

OK.

[schomatis]

SGTM

[kevina]

This SGTM but I would need to see how the implementation works out.

[schomatis]

@Stebalien Is this blocked by #5634 or can @chenminjian still attempt a fix here?

[Stebalien]

Not really. That fixes the thread-safety issue but still doesn't allow for atomic config updates. For that, we need this change (so we can apply transformations while holding a lock on the entire config.

[Stebalien]

Really, I'm not sure if there is a thread-safety issue anymore (if ever?). However, having a way to clone the config is still useful.

[schomatis]

However, having a way to clone the config is still useful.

Ok, pinging @chenminjian then, as he seemed really interested in this and has already landed some pretty useful patches regarding the go-ipfs config system.

[Stebalien]

Digging that PR back up, it looks like it addressed two separate issues:

1. It deep-clones the config before updating it. This probably wasn't necessary as we (a) dereference it and then (b) replace (not modify) mutable datastructures.
2. It replaces the stored config by pointer instead of by-value. This was the actual data race.

[qiwaa]

❤️ Thanks for pinging me. I watched go-ipfs, so I didn't notice this email immediately.

[qiwaa]

However, having a way to clone the config is still useful.

This has been implemented in the latest go-ipfs-config.