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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jan 5 09:36:37 PST 2009

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

Attachment 26432: proposed fix

------- 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.


More information about the webkit-reviews mailing list