Make RuntimeSettings.Pref thread safe
Categories
(GeckoView :: General, defect, P2)
Tracking
(Not tracked)
People
(Reporter: mcomella, Unassigned)
References
(Blocks 1 open bug)
Details
Attachments
(4 obsolete files)
Break-out from bug 1750896 – I was in a good position to fix this portion of the bug so I took a stab at it.
Reporter | ||
Comment 1•3 years ago
|
||
Note: an alternative to making this class thread safe is to confine usage of the Pref class to specific threads only. If we decide to go that route in bug 1750896, this patch isn't needed (though maybe we'd want to use the ConcurrentTestRule elsewhere).
Reporter | ||
Comment 2•3 years ago
|
||
Reporter | ||
Comment 3•3 years ago
|
||
This test is @Ignore in this commit because it introduces an intentional test
failure that uncovers the concurrency bug.
Depends on D136546
Reporter | ||
Comment 4•3 years ago
|
||
The class is technically not thread safe yet because a reference to the
unsynchronized class is leaked from the constructor: this will be addressed in
the next commit for clarity.
Depends on D136547
Reporter | ||
Comment 5•3 years ago
|
||
This completes making RuntimeSettings.Pref thread safe.
As far as I can tell in the current implementation, the values passed to Pref
(of type T) are all thread safe because they are immutable so there are no
additional concurrency bugs created by those values.
Depends on D136548
Updated•3 years ago
|
Reporter | ||
Comment 6•3 years ago
|
||
Fixing concurrency bugs isn't my main focus so I'm concerned changing the concurrency test to fit existing GV patterns would take too long for me. We could land the code without the test or someone can pick up where I left off on the test.
I'm not actively working on moving this forward so unassigning.
Comment hidden (off-topic) |
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Reporter | ||
Comment 8•2 years ago
|
||
Without checking the code, I assume ths bug is still relevant but I'm too far away from the code to continue to work on this. My patches are probably still relevant too but they've probably bitrotted a bit.
Reporter | ||
Updated•2 years ago
|
Description
•