Summary: | add assertions to check Font objects are used only on a single thread | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Cameron McCormack (:heycam) <heycam> | ||||
Component: | Layout and Rendering | Assignee: | Cameron McCormack (:heycam) <heycam> | ||||
Status: | NEW --- | ||||||
Severity: | Normal | CC: | bfulgham, clord, mmaxfield, simon.fraser, webkit-bug-importer, zalan | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Local Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Bug Depends on: | |||||||
Bug Blocks: | 233488 | ||||||
Attachments: |
|
Description
Cameron McCormack (:heycam)
2021-12-07 21:18:19 PST
Created attachment 446287 [details] WIP Here's an incomplete patch that shows one approach, using the bits I'm adding in bug 233967. Unfortunately it's pretty noisy since the WTF_GUARDED_BY_LOCK attribute needs to be added on every member variable. An alternative would be to bundle up all the data members into a struct and have a single member variable with WTF_GUARDED_BY_LOCK on it, but that would cause corresponding noise on all member variable references. Sadly wrapping all the data members in an anonymous struct prevents WTF_GUARDED_BY_LOCK from being used on it in a single place. (It would be nice if a single annotation could be added on the class itself.) I think the title is wrong? This seems a bit overkill to me vs just guarding on the functions that provide Font objects - there's no point where we transfer Fonts between threads or access Fonts from different threads from the FontCache that provided them are there? Guarding on the places that provide the Font objects might be a better balance between safety (and ability to rely on the static analysis attributes) and noisiness. And you're right, I don't think we do explicitly transfer Fonts between threads. I guess I just want to protect against future code changes that might provide a different way to get access to Font objects. A different approach could be to have some handle class that wraps a raw Font*, asserts in its operator->, and doesn't provide a way to get at the raw pointer. The static constructor functions are the only way to create Font instances and would return Ref<FontHandle>. |