Closed Bug 1436916 Opened 2 years ago Closed 2 years ago

It's likely that many VarCache global variables are racy

Categories

(Core :: Preferences: Backend, enhancement)

enhancement
Not set

Tracking

()

RESOLVED WORKSFORME
Tracking Status
firefox60 --- affected

People

(Reporter: njn, Unassigned)

References

Details

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.
See bug 1465852, which has a patch for this.
Depends on: 1465852
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.)
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.