Bug 228108 - Protect m_data when iterating in CachedRawResource::didAddClient
Summary: Protect m_data when iterating in CachedRawResource::didAddClient
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-07-20 09:03 PDT by Alex Christensen
Modified: 2021-09-09 13:26 PDT (History)
4 users (show)

See Also:


Attachments
Patch (1.79 KB, patch)
2021-07-20 09:06 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (2.01 KB, patch)
2021-07-20 09:18 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2021-07-20 09:03:45 PDT
Protect m_data when iterating in CachedRawResource::didAddClient
Comment 1 Alex Christensen 2021-07-20 09:06:30 PDT
Created attachment 433876 [details]
Patch
Comment 2 Alex Christensen 2021-07-20 09:06:33 PDT
<rdar://problem/80716674>
Comment 3 Chris Dumez 2021-07-20 09:12:08 PDT
Comment on attachment 433876 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=433876&action=review

r=me with optional nit fixes.

> Source/WebCore/loader/cache/CachedRawResource.cpp:170
>              if (m_data) {

We could also just do:
if (auto data = m_data) {

and use data below instead of m_data.

> Source/WebCore/loader/cache/CachedRawResource.cpp:171
> +                auto protectedData = RefPtr { m_data };

instead of adding the protector here.

> Source/WebCore/loader/cache/CachedRawResource.cpp:173
> +                    if (!hasClient(*client))

Better safe than sorry indeed.

I know we like early return in WebKit, but when it's one line life this, I don't feel like it's great.
Comment 4 Alex Christensen 2021-07-20 09:18:58 PDT
Created attachment 433877 [details]
Patch
Comment 5 EWS 2021-07-20 09:55:05 PDT
Committed r280083 (239808@main): <https://commits.webkit.org/239808@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 433877 [details].
Comment 6 Chris Dumez 2021-09-09 13:10:06 PDT
Sadly, it seems this didn't do the trick :( <rdar://82936913>