Summary: | ConservativeRoots triggers page demand on Speedometer | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Geoffrey Garen <ggaren> | ||||||
Component: | New Bugs | Assignee: | Geoffrey Garen <ggaren> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, ews-watchlist, fpizlo, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | 225705 | ||||||||
Bug Blocks: | |||||||||
Attachments: |
|
Description
Geoffrey Garen
2021-05-11 15:54:03 PDT
Created attachment 428319 [details]
Patch
Comment on attachment 428319 [details]
Patch
cq+
Committed r277346 (237604@main): <https://commits.webkit.org/237604@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 428319 [details]. Comment on attachment 428319 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=428319&action=review > Source/JavaScriptCore/heap/ConservativeRoots.cpp:61 > + m_roots.append(bitwise_cast<HeapCell*>(p)); Doesn't this code while some threads are thread_suspended? In that case, I think you just introduced one of my favorite deadlocks: - The append() calls malloc, which tries to grab some lock. - Meanwhile some other thread is thread_suspended while holding that lock. Nothing says that thread_suspend can't stop a thread while it's inside fastMalloc, system malloc, or anything else that holds locks. I think that's why this code previously used a super janky want of allocating memory -- it was that way so that it didn't have to take locks to allocate. Re-opened since this is blocked by bug 225705 Created attachment 428405 [details]
Patch
Comment on attachment 428405 [details]
Patch
cq+
Committed r277388 (237644@main): <https://commits.webkit.org/237644@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 428405 [details]. |