| Summary: | Worklist::setNumberOfThreads seems incompatible with how we lock elsewhere | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Saam Barati <saam> |
| Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> |
| Status: | NEW --- | ||
| Severity: | Normal | CC: | benjamin, fpizlo, ggaren, gskachkov, guijemont, keith_miller, mark.lam, msaboff, rmorisset, saam, ticaiolima, tzagallo, ysuzuki |
| Priority: | P2 | ||
| Version: | WebKit Nightly Build | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
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); } } ```