Not doing this resulted in a runtime assertion failure due to a MarkedBlock being in the FreeListed state while releaseExecutableMemory() is calling Heap::getConservativeRegisterRoots(). This was encountered while testing script timeout code (which uses releaseExecutableMemory()). Patch forthcoming.
Created attachment 198500 [details] the patch.
Comment on attachment 198500 [details] the patch. r=me. Maybe we should add an assertion that the Heap has been canonicalized whenever we're doing anything with root scanning (i.e. inside getConservativeRegisterRoots, gatherConservativeRoots, etc.)
(In reply to comment #2) > (From update of attachment 198500 [details]) > r=me. Maybe we should add an assertion that the Heap has been canonicalized whenever we're doing anything with root scanning (i.e. inside getConservativeRegisterRoots, gatherConservativeRoots, etc.) i'd r+ patch that did that
(In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 198500 [details] [details]) > > r=me. Maybe we should add an assertion that the Heap has been canonicalized whenever we're doing anything with root scanning (i.e. inside getConservativeRegisterRoots, gatherConservativeRoots, etc.) > > i'd r+ patch that did that How does one assert that the heap has be canonicalized without actually doing a canonicalizing pass?
Thanks for the review. Landed in r148616: <http://trac.webkit.org/changeset/148616>.
(In reply to comment #4) > (In reply to comment #3) > > (In reply to comment #2) > > > (From update of attachment 198500 [details] [details] [details]) > > > r=me. Maybe we should add an assertion that the Heap has been canonicalized whenever we're doing anything with root scanning (i.e. inside getConservativeRegisterRoots, gatherConservativeRoots, etc.) > > > > i'd r+ patch that did that > > How does one assert that the heap has be canonicalized without actually doing a canonicalizing pass? No idea, that's why i said i'd r+ it rather than write it :D
(In reply to comment #4) > (In reply to comment #3) > > (In reply to comment #2) > > > (From update of attachment 198500 [details] [details] [details]) > > > r=me. Maybe we should add an assertion that the Heap has been canonicalized whenever we're doing anything with root scanning (i.e. inside getConservativeRegisterRoots, gatherConservativeRoots, etc.) > > > > i'd r+ patch that did that > > How does one assert that the heap has be canonicalized without actually doing a canonicalizing pass? There's a number of ways. One would be to add a flag that states whether the Heap (or the MarkedSpace) is in the "Canonicalized" state. You would then set this flag whenever canonicalize gets called and unset it whenever allocation caused us to leave the "Canonicalized" state. You could also just ask all of the MarkedAllocators if their internal state matches that of being canonicalized (i.e. their FreeLists are empty and their m_currentBlock == 0).