[webkit-reviews] review denied: [Bug 22614] Need to add Win32 implementation of ThreadSpecific. : [Attachment 25697] Proposed Patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has denied Jian Li <jianli at chromium.org>'s
request for review:
Bug 22614: Need to add Win32 implementation of ThreadSpecific.
https://bugs.webkit.org/show_bug.cgi?id=22614

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

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
> -#if USE(PTHREADS) || PLATFORM(WIN)
> +#if USE(PTHREADS) || PLATFORM(WIN) || PLATFORM(WIN_OS)

No need to keep PLATFORM(WIN), because it can only be set if PLATFORM(WIN_OS)
is set.

> +#if USE(PTHREADS) || PLATFORM(WIN)
>      pthread_key_t m_key;

Similarly, I think this test should be just USE(PTHREADS).

> +template<typename T>
> +inline void ThreadSpecific<T>::destroy(void* ptr)

What calls destroy() when a thread exits? Per
<http://msdn.microsoft.com/en-us/library/ms686801(VS.85).aspx>, data kept in
per-thread storage must be destroyed manually in a DLL_THREAD_DETACH callback.

r-, because I think that per-thread data leaks.