Bug 196383 - Worklist::setNumberOfThreads seems incompatible with how we lock elsewhere
Summary: Worklist::setNumberOfThreads seems incompatible with how we lock elsewhere
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-03-28 17:32 PDT by Saam Barati
Modified: 2019-03-28 17:33 PDT (History)
13 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 2019-03-28 17:32:59 PDT
E.g, there is no way this is right anymore:
```
void Worklist::visitWeakReferences(SlotVisitor& visitor)
{
    VM* vm = visitor.heap()->vm();
    {
        LockHolder locker(*m_lock);
        for (PlanMap::iterator iter = m_plans.begin(); iter != m_plans.end(); ++iter) {
            Plan* plan = iter->value.get();
            if (plan->vm() != vm)
                continue;
            plan->checkLivenessAndVisitChildren(visitor);
        }
    }
    // This loop doesn't need locking because:
    // (1) no new threads can be added to m_threads. Hence, it is immutable and needs no locks.
    // (2) ThreadData::m_safepoint is protected by that thread's m_rightToRun which we must be
    //     holding here because of a prior call to suspendAllThreads().
    for (unsigned i = m_threads.size(); i--;) {
        ThreadData* data = m_threads[i].get();
        Safepoint* safepoint = data->m_safepoint;
        if (safepoint && safepoint->vm() == vm)
            safepoint->checkLivenessAndVisitChildren(visitor);
    }
}
```