[Mesa-dev] [PATCH 3/3] c11/threads: Don't implement thrd_current on Windows.

[Brian Paul]

On 03/03/2014 02:41 PM, jfonseca at vmware.com wrote:
> From: José Fonseca <jfonseca at vmware.com>
>
> GetCurrentThread() returns a pseudo-handle (a constant which can only
> be used within the calling thread) and not a real handle.
>
> DuplicateHandle will return a real handle, but it will create a new handle
> every time we call.  Calling
> DuplicateHandle here means we will leak handles, which is also very bad.

Can you fix the word wrapping in that paragraph?

Otherwise, the series LGTM.  Reviewed-by: Brian Paul <brianp at vmware.com>

And these could probably go into the next stable release(s):

Cc: "10.0" "10.1" <mesa-stable at lists.freedesktop.org>


>
> In short, the Windows implementation of thrd_t needs a thorough make over.
> And it won't be pretty.  It looks like C11 committee over-simplified
> things: it would be much better to have seperate objects for threads and
> thread IDs like C++11.
>
> For now, just comment out the thrd_current() implementation, so we get
> build errors if anybody tries to use them.
>
> Thanks to Brian Paul for spotting and diagnosing this problem.
> ---
>   include/c11/threads_win32.h | 34 ++++++++++++++++++++++++++++++++--
>   1 file changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/include/c11/threads_win32.h b/include/c11/threads_win32.h
> index 771db94..d4c2a72 100644
> --- a/include/c11/threads_win32.h
> +++ b/include/c11/threads_win32.h
> @@ -492,12 +492,42 @@ thrd_create(thrd_t *thr, thrd_start_t func, void *arg)
>       return thrd_success;
>   }
>
> +#if 0
>   // 7.25.5.2
>   static inline thrd_t
>   thrd_current(void)
>   {
> -    return GetCurrentThread();
> +    HANDLE hCurrentThread;
> +    BOOL bRet;
> +
> +    /* GetCurrentThread() returns a pseudo-handle, which is useless.  We need
> +     * to call DuplicateHandle to get a real handle.  However the handle value
> +     * will not match the one returned by thread_create.
> +     *
> +     * Other potential solutions would be:
> +     * - define thrd_t as a thread Ids, but this would mean we'd need to OpenThread for many operations
> +     * - use malloc'ed memory for thrd_t. This would imply using TLS for current thread.
> +     *
> +     * Neither is particularly nice.
> +     *
> +     * Life would be much easier if C11 threads had different abstractions for
> +     * threads and thread IDs, just like C++11 threads does...
> +     */
> +
> +    bRet = DuplicateHandle(GetCurrentProcess(), // source process (pseudo) handle
> +                           GetCurrentThread(), // source (pseudo) handle
> +                           GetCurrentProcess(), // target process
> +                           &hCurrentThread, // target handle
> +                           0,
> +                           FALSE,
> +                           DUPLICATE_SAME_ACCESS);
> +    assert(bRet);
> +    if (!bRet) {
> +	hCurrentThread = GetCurrentThread();
> +    }
> +    return hCurrentThread;
>   }
> +#endif
>
>   // 7.25.5.3
>   static inline int
> @@ -511,7 +541,7 @@ thrd_detach(thrd_t thr)
>   static inline int
>   thrd_equal(thrd_t thr0, thrd_t thr1)
>   {
> -    return (thr0 == thr1);
> +    return GetThreadId(thr0) == GetThreadId(thr1);
>   }
>
>   // 7.25.5.5
>