[Mesa-dev] [PATCH 1/2] c11/threads: implement thrd_current on Windows

[Jose Fonseca]

No, I'm afraid the problem is still there AFAICS.

GetCurrentThread returns a magic constant (-2 if I'm not mistaken).  So 
it is (and always will be) useless for thrd_current.

Imagine that:

- thread #1 invokes thrd_current() and gets -2
- thread #2 invokes thrd_current() and gets -2
- a thread (#1, #2 or another thrad) receives invokes thread_equal(-2, -2)


The only way this would work is if it would be forbidden to pass the 
value of thrd_current to other threads.  I'll need to re-read the specs, 
but my guess is that it would be pretty useless.

I also doubt the way mapi uses would work.


Furthermore map really wants a fast thread id.  It doesn't need actual 
thread handles, just some way of telling threads apart, any id.


Jose


On 20/04/17 15:15, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov at collabora.com>
>
> Earlier commit removed the implementation due to a subtle bug, in the
> existing code. Namely:
>
> One was comparing directly the thread handle/IDs rather than using
> thrd_equal(). As the pseudo-handles never matched things went south.
>
> That bug was resolved with commit 458c7490c29 "mapi: rewrite
> u_current_init() function without u_thread_self()" thus we can bring the
> thrd_current implementation back, whist preserving the correct
> thrd_equal implementation as-is.
>
> With this in place, we can remove a handful of the remaining pthread vs
> WIN32 specifics.
>
> Cc: Brian Paul <brianp at vmware.com>
> Cc: José Fonseca <jfonseca at vmware.com>
> Signed-off-by: Emil Velikov <emil.l.velikov at gmail.com>
> ---
>  include/c11/threads_win32.h | 35 ++++++-----------------------------
>  1 file changed, 6 insertions(+), 29 deletions(-)
>
> diff --git a/include/c11/threads_win32.h b/include/c11/threads_win32.h
> index d017c31c34e..f0eb5c77c60 100644
> --- a/include/c11/threads_win32.h
> +++ b/include/c11/threads_win32.h
> @@ -494,42 +494,19 @@ 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)
>  {
> -    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.
> +    /* GetCurrentThread() returns a pseudo-handle, which will not match the
> +     * one returned by thread_create.
>       *
> -     * Life would be much easier if C11 threads had different abstractions for
> -     * threads and thread IDs, just like C++11 threads does...
> +     * At the same time, the only way that one should compare thread handle
> +     * and/or IDs is via thrd_equal. That in itself attributes for the above
> +     * situation.
>       */
> -
> -    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;
> +    return GetCurrentThread();
>  }
> -#endif
>
>  // 7.25.5.3
>  static inline int
>