Bug 247288 - Change m_node in RenderObject to being a WeakPtr
Summary: Change m_node in RenderObject to being a WeakPtr
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chirag M Shah
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-10-31 14:09 PDT by Chirag M Shah
Modified: 2022-11-04 13:04 PDT (History)
6 users (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Chirag M Shah 2022-10-31 14:09:17 PDT 
m_node should be a WeakPtr instead of a plain C++ reference so that we don't hit use-after-free and instead crash.
Comment 1 Simon Fraser (smfr) 2022-10-31 14:41:55 PDT 
WeakPtr is not free; there are additional memory and performance costs because of the back-referencing required. We should do some memory and perf testing before landing this.
Comment 2 Ryosuke Niwa 2022-10-31 15:33:08 PDT 
We should also explore if CheckedRef is a better alternative. It prevents UAF of free'd memory and it's slightly cheaper than WeakPtr in terms of instantiation (no extra malloc) and dereference (no chained indirect loads). Node currently doesn't support CheckedPtr/CheckedRef though so we'd need to figure that one out but if WeakPtr ended up causing a perf regression or semantics of reference makes more sense, then we should consider using CheckedRef.
Comment 3 EWS 2022-11-03 11:36:52 PDT 
Committed 256282@main (63c86a3d1b18): <https://commits.webkit.org/256282@main>

Reviewed commits have been landed. Closing PR #5977 and removing active labels.
Comment 4 Radar WebKit Bug Importer 2022-11-03 11:37:21 PDT 
<rdar://problem/101922970>
Comment 5 David Kilzer (:ddkilzer) 2022-11-04 13:04:29 PDT 
Corrected radar:

<rdar://101505011>