[webkit-reviews] review denied: [Bug 77488] Replace JSArray destructor with finalizer : [Attachment 124854] Patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has denied Mark Hahnenberg
<mhahnenberg at apple.com>'s request for review:
Bug 77488: Replace JSArray destructor with finalizer
https://bugs.webkit.org/show_bug.cgi?id=77488

Attachment 124854: Patch
https://bugs.webkit.org/attachment.cgi?id=124854&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=124854&action=review


> Source/JavaScriptCore/runtime/JSArray.cpp:314
> +    if (!map) {
>	   map = m_sparseValueMap = new SparseArrayValueMap;
> +	   globalData.heap.addFinalizer(this, finalize);

m_sparseValueMap can oscillate between null and non-null. In such a case, this
code will register one finalizer for each oscillation, which will result in a
double delete.

Please write a test case for this condition.

You can fix this by setting thisObject->m_sparseValueMap to 0 after deleting
it, adding a comment that the finalizer can run more than once, or by never
setting m_sparseValueMap to 0, even if it becomes empty.