-
fpizlo@apple.com authored
https://bugs.webkit.org/show_bug.cgi?id=121661 Reviewed by Mark Hahnenberg. This method shouldn't have been called from the concurrent JIT thread. That's hard to prevent so I added a return-early check using isCompilationThread(). Here's why this makes sense. Structure has two ways to tell you about the layout of the objects it is describing: m_offset and the property table. Most structures only have m_offset and report null for the property table. If the property table is there, it will tell you additional information and that information subsumes m_offset - but the m_offset is still there. So, when we have a property table, we have to keep it in sync with the m_offset. There is a bunch of machinery to do this. Changing the property table only happens on the main thread. Because the machinery to change the property table is so complex, especially with respect to keeping it in sync with m_offset, we have the checkOffsetConsistency method. It's meant to be called at key points before and after changes to the property table or the offset. Most clients of Structure who care about object layout, including the concurrent thread, will want to know m_offset and not the property table. If they want the property table, they will already be super careful. The concurrent thread has special methods for this, like Structure::getConcurrently(), which uses fine-grained locking to ensure that it sees a coherent view of the property table. Adding locking to checkOffsetConsistency() is probably a bad idea since that method may be called when the relevant lock is already held. So, we'd have awkward recursive locking issues. But right now, the concurrent JIT thread may call a method, like Structure::outOfLineCapacity(), which has a call to checkOffsetConsistency(). The call to checkOffsetConsistency() is there because we have found that it helps quickly identify situations where the property table and m_offset get out of sync - mainly because code that changes either of those things will usually also want to know the outOfLineCapacity(). But Structure::outOfLineCapacity() doesn't *actually* need the property table; it uses the m_offset. The concurrent JIT is correct to call outOfLineCapacity(), and is right to do so without holding any locks (since in all cases where it calls outOfLineCapacity() it has already proven that m_offset is immutable). But because outOfLineCapacity() calls checkOffsetConsistency(), and checkOffsetConsistency() doesn't grab locks, and that same structure is having its property table modified by the main thread, we end up with these spurious assertion failures. FWIW, the structure isn't *actually* having *its* property table modified - instead what happens is that some downstream structure steals the property table and then starts adding things to it. The concurrent thread loads the property table before it's stolen, and hence the badness. I suspect there are other code paths that lead to the concurrent JIT calling some Structure method that it is fine and safe to call, but then that method calls checkOffsetConsistency(), and then you have a possible crash. The most sensible solution to this appears to be to make sure that checkOffsetConsistency() is aware of its uselessness to the concurrent JIT thread. This change makes it return early if it's in the concurrent JIT. * runtime/StructureInlines.h: (JSC::Structure::checkOffsetConsistency): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@157645 268f45cc-cd09-0410-ab3c-d52691b4dbfc
d5cc6107