Bug 225676 – ConservativeRoots triggers page demand on Speedometer

• Component: New Bugs
• Status: RESOLVED FIXED

[Geoffrey Garen]

ConservativeRoots triggers page demand on Speedometer

[Geoffrey Garen]

Created attachment 428319 [details]
Patch

[Geoffrey Garen]

Comment on attachment 428319 [details]
Patch

cq+

[EWS]

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].

[Radar WebKit Bug Importer]

<rdar://problem/77872621>

[Filip Pizlo]

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.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 225705

[Geoffrey Garen]

Created attachment 428405 [details]
Patch

[Geoffrey Garen]

Comment on attachment 428405 [details]
Patch

cq+

[EWS]

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].