Add class that works with clang thread safety annotations to assert we're running on a specific event target
Categories
(Core :: XPCOM, task)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox102 | --- | fixed |
People
(Reporter: bryce, Assigned: bryce)
References
Details
Attachments
(1 file)
A common pattern in media code (and I assume elsewhere) is to restrict access to certain data and functions to a single (serial) event target. The logic typically being, if member foo is only accessed on a single serial event target, then we cannot race when accessing it. This then generalizes to functions that access foo, they all too must be called on the appropriate event target to ensure safety.
A convention for such code is to assert that it's on the correct event target when it begins, such as here in the TrackBuffersManager. This ensures at run time that the code is being used as intended.
Now that we have thread safety analysis annotations, I think we can do even better. We could create a lock-able like class that asserts it's on the appropriate event target, and use annotations to ensure that we call the assertion code. This has a similar benefit to using the annotations with traditional locks in that it forces us to do the right thing once the annotations are in place. I.e. just as annotations make us take a lock before accessing data guarded by a lock, so too could they make us assert we're on the right target before accessing data that should only be accessed on that target.
This wouldn't catch all issues at compile time. Users could assert they're on the wrong target and cause a run time assertion. But it would do a couple of things that I consider useful
- Document what members are expected to be used only on a specific target and enforce that some assertion is done around this.
- Avoid the foot gun where it's not clear what target a member should be accessed on, so folks just access them, then we only catch issues later due to run time analysis or sec bugs.
|
Assignee | |
Comment 1•4 years ago
|
||
Add helper class that works with Clang's thread safety analysis to ensure
event target asserts are called.
The Media codebase has a lot of code that expects access to members to only
happen on a specific target, and something like this would be useful to ensure
that asserts are run as expected, as well as warning users who write new code
that me be doing off-target work.
|
|
Assignee | |
Comment 2•4 years ago
|
||
I have some WIP for some media code that uses the proposed EventTargetAsserter. I'll try and get it pushed later today to give a further example of how I think this could be handy.
|
|
Assignee | |
Comment 3•4 years ago
|
||
Bug 1767899 shows an example of how I envision this could be used.
|
||
Updated•4 years ago
|
|
||
Comment 5•4 years ago
|
||
| bugherder | ||
Description
•