1436916 - It's likely that many VarCache global variables are racy

Status: RESOLVED WORKSFORME

(Core :: Preferences: Backend, enhancement)

Description

[Nicholas Nethercote [inactive]]

VarCache prefs have a global variable that holds a cached copy of the pref's current value. This is useful for two reasons:

- A global variable access is faster than a hash table lookup.

- A global variable can be accessed off the main thread, whereas libpref's hash table is main-thread only.

For prefs that take advantage of the off-main-thread accesses, the global variable should be atomic, because the main thread will write the global variable whenever the pref is updated. This is the case for some VarCache prefs, but it's very likely that some global variables that should be atomic are not. Particularly given that a lot of them are floats, and Atomic<float> isn't available.

Currently the only way to detect these cases is via code inspection. However, bug  1436655 will enable automatic checking, by introducing getters for all VarCache global variables; these getters can include thread checks.

The effects of the insufficient synchronization are highly unpredictable, and will depend on the platform. Pref updates are rare enough in practice that problems are likely to show up only rarely.

Comment 1

[Nicholas Nethercote [inactive]]

See bug 1465852, which has a patch for this.

Comment 2

[Nicholas Nethercote [inactive]]

Bug 1465852 has landed. We still don't have a good story for float prefs, because Atomic<float> isn't a thing. I wonder if we can do something hacky: use Atomic<int32_t> and then cast to/from float when getting/setting. (We do something similar with normal float prefs, which are actually represented internally as strings.)