Summary: | FetchResponse::BodyLoader should not be movable | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Zan Dobersek <zan> | ||||||
Component: | New Bugs | Assignee: | Zan Dobersek <zan> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | achristensen, ews-watchlist, webkit-bug-importer, youennf | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Zan Dobersek
2019-07-01 14:11:53 PDT
This is currently causing a few crashes in fetch-related tests on debug bots using GCC, with "pure virtual method" error being thrown when calling FetchLoaderClient virtual methods on already-freed FetchResponse::BodyLoader objects. Created attachment 373265 [details]
Patch
Attachment 373265 [details] did not pass style-queue:
ERROR: Source/WebCore/Modules/fetch/FetchResponse.h:37: Alphabetical sorting problem. [build/include_order] [4]
ERROR: Source/WebCore/ChangeLog:12: Please consider whether the use of security-sensitive phrasing could help someone exploit WebKit: use-after-free [changelog/unwantedsecurityterms] [3]
Total errors found: 2 in 3 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 373265 [details]
Patch
Classic case of moved-from undefined state bug. We still need to be careful about std::exchange/WTFMove being used correctly.
I think we should make m_bodyLoader a std::unique_ptr instead of an Optional. Created attachment 373305 [details]
Patch
Takes the unique_ptr approach.
Comment on attachment 373305 [details] Patch Clearing flags on attachment: 373305 Committed r247087: <https://trac.webkit.org/changeset/247087> All reviewed patches have been landed. Closing bug. |