| Summary: | CrashTracer: com.apple.WebKit.WebContent at JavaScriptCore: bmalloc_allocate_impl_impl_slow | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Jean-Yves Avenard [:jya] <jean-yves.avenard> | ||||||||||||||
| Component: | New Bugs | Assignee: | Jean-Yves Avenard [:jya] <jean-yves.avenard> | ||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||
| Severity: | Normal | CC: | cdumez, ews-watchlist, japhet, jer.noble, mifenton, mmaxfield, webkit-bug-importer | ||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||
| Version: | Other | ||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||
| OS: | Unspecified | ||||||||||||||||
| Bug Depends on: | |||||||||||||||||
| Bug Blocks: | 236736 | ||||||||||||||||
| Attachments: |
|
||||||||||||||||
|
Description
Jean-Yves Avenard [:jya]
2022-02-16 01:40:10 PST
Created attachment 452161 [details]
WIP
Created attachment 452169 [details]
WIP
Created attachment 452172 [details]
WIP
Created attachment 452222 [details]
Patch
Comment on attachment 452222 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=452222&action=review > Source/WebCore/ChangeLog:11 > + FragmentedSharedBuffer is receieved from the network process, it is proactively coalesced received > Source/WebCore/loader/cache/CachedRawResource.cpp:118 > + if (auto incrementalData = calculateIncrementalDataChunk(contiguousData)) { this should be using data rather than contiguousData. Otherwise the client would have received each individual segment, and then a SharedBufferDataView restricting the content to just the last segment received but that actually takes a reference to the entire content. This could potentially means we keep a reference for longer than needed. > Source/WebCore/loader/cache/CachedScript.cpp:87 > m_scriptHash = m_script.impl()->hash(); If the CachedScript received fragmented content, finishLoading would have been called. This version has dropped the makeContiguous to m_data in finishLoading, this could potentially lead to multiple call to makeContiguous on the same buffer (In reply to Jean-Yves Avenard [:jya] from comment #6) > Comment on attachment 452222 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=452222&action=review > > > Source/WebCore/ChangeLog:11 > > + FragmentedSharedBuffer is receieved from the network process, it is proactively coalesced > > received > > > Source/WebCore/loader/cache/CachedRawResource.cpp:118 > > + if (auto incrementalData = calculateIncrementalDataChunk(contiguousData)) { > > this should be using data rather than contiguousData. > > Otherwise the client would have received each individual segment, and then a > SharedBufferDataView restricting the content to just the last segment > received but that actually takes a reference to the entire content. This > could potentially means we keep a reference for longer than needed. Okay. Can we fix that in a future patch? This one keeps the original behavior of coalescing and storing in m_data, then passing m_data in. > > Source/WebCore/loader/cache/CachedScript.cpp:87 > > m_scriptHash = m_script.impl()->hash(); > > If the CachedScript received fragmented content, finishLoading would have > been called. This version has dropped the makeContiguous to m_data in > finishLoading, this could potentially lead to multiple call to > makeContiguous on the same buffer Not sure I understand here; CachedScript::finishLoading() still coalesces the incoming buffer. Comment on attachment 452222 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=452222&action=review > Source/WebCore/loader/cache/CachedRawResource.cpp:70 > + m_data = data.copy(); forgot to mention, if you take a copy rather than a reference to data ; you must keep a strong ref to data above, as a call to notifyClientsDataWasReceived will result in a re-entrant call to the CachedRawResource parent that will destroy data. Created attachment 452294 [details]
Patch
Created attachment 452295 [details]
Patch
Committed r290005 (247391@main): <https://commits.webkit.org/247391@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 452295 [details]. |