[webkit-reviews] review granted: [Bug 23073] (r39465) Workers crash when running raytracer : [Attachment 26432] proposed fix

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Alexey Proskuryakov <ap at webkit.org>'s
request for review:
Bug 23073: (r39465) Workers crash when running raytracer
https://bugs.webkit.org/show_bug.cgi?id=23073

Attachment 26432: proposed fix
https://bugs.webkit.org/attachment.cgi?id=26432&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +	   (WTF::::destroy): Changed to clear the pointer only after data
object destruction is
> +	   finished - otherwise, WebCore::ThreadGlobalData destructor was
re-creating the object
> +	   in order to access atomic string table.

The prepare-ChangeLog script generated a bad function name here.

> +	   (WTF::operator T*): Symmetrically, set up the per-thread pointer
before data constructor
> +	   is called.

And here. Human intervention is required!

> +    data->value->~T();
> +    fastFree(data->value);
>      pthread_setspecific(data->owner->m_key, 0);
> -    delete data->value;

I think this really needs a comment. It's quite non-obvious that the ordering
is important from  looking at the code.

> -	   ptr = new T();
> +	   ptr = static_cast<T*>(fastMalloc(sizeof(T)));
>	   set(ptr);
> +	   new (ptr) T;

Same thing goes here, but even more so. You need to justify why it was
necessary to use placement new here.

Change looks great.

r=me