[Mesa-dev] [RFC 1/2] Revert "mapi: rewrite u_current_init() function without u_thread_self()"

[Jose Fonseca]

On 20/04/17 10:39, Emil Velikov wrote:
> On 20 April 2017 at 00:36, Timothy Arceri <tarceri at itsqueeze.com> wrote:
>> On 19/04/17 20:32, Emil Velikov wrote:
>>>
>>> On 19 April 2017 at 08:45, Timothy Arceri <tarceri at itsqueeze.com> wrote:
>>>>
>>>> This reverts commit 458c7490c29ef2960a33a089f65490e044da5d27.
>>>>
>>>> The commit did not revert cleanly so this was fixed up by hand.
>>>>
>>>> u_thread_self() will be used by the following patch.
>>>
>>> I see your interesting in dropping the legacy/unused extension with
>>> 2/2 but this commit is bonkers :-(
>>
>>
>> This just reverts the code to how it used to be.
>>
> Bringing bugs back is never a good idea.
>
>>>
>>>
>>>> -   else if (!thread_id_equal(knownID, get_thread_id())) {
>>>> +   else if (knownID != u_thread_self()) {
>>>
>>> NACK. Direct comparison as seen here is broken and if it works that's
>>> only by coincidence.
>>
>>
>> In what way is it broken?
>>
> To quote the pthread manual:
>
> pthread_self
> ... Therefore, variables of type pthread_t can't portably be compared
> using the C equality operator (==); use pthread_equal(3) instead.
>
> pthread_equal
> ...The  pthread_equal() function is necessary because thread IDs
> should be considered opaque: there is no portable way for applications
> to directly compare two pthread_t values.
>
> On the Windows side things are somewhat similar.
>
> -Emil

I agree -- we should be using pthread_equal --, but I'm not sure if 
using thread_current/thread_equal is a good idea long term, as we're not 
here if these are lightweight operations.  They are for Linux on our 
current threads.h implementation, but Windows definetely is not.

So it might make sense to never use threads.h and directly use 
pthreads/winthreads here.

Jose