search

joyent / libuv

Go to
https://github.com/libuv/libuv
3.27k stars 653 forks source link

win: fix uv_thread_self() #1564

Closed orangemocha closed 10 years ago

orangemocha commented 10 years ago

59658a8de7cc05a58327a164fd2ed4b050f8b4f4 changed uv_thread_self() to return uv_thread_t, but uv_thread_t is a thread's HANDLE while uv_thread_self() returns the current thread's id. This means that uv_thread_equal() is also broken, as we are potentially comparing HANDLES to ids.

Changed uv_thread_t to DWORD. Fixed small doc issue.

orangemocha commented 10 years ago

This will need some follow up because now we are leaking the HANDLE return by _beginthreadex

piscisaureus commented 10 years ago

@orangemocha @saghul My suggestion would be to partially revert https://github.com/joyent/libuv/commit/59658a8de7cc05a58327a164fd2ed4b050f8b4f4

uv_thread_self() should not return a HANDLE but just the thread id. uv_thread_equal() should just compare two unsigned longs on windows.

saghul commented 10 years ago

I'm ok with that. In the end we treat uv_thread_t as opaque, so it shouldn't matter much.

On Friday, November 7, 2014, Bert Belder notifications@github.com wrote:

@orangemocha https://github.com/orangemocha @saghul https://github.com/saghul My suggestion would be to partially revert 59658a8 https://github.com/joyent/libuv/commit/59658a8de7cc05a58327a164fd2ed4b050f8b4f4

uv_thread_self() should not return a HANDLE but just the thread id. uv_thread_equal() should just compare two unsigned longs on windows.

— Reply to this email directly or view it on GitHub https://github.com/joyent/libuv/pull/1564#issuecomment-62197647.

/Saúl bettercallsaghul.com

orangemocha commented 10 years ago

uv_thread_self() should not return a HANDLE but just the thread id. uv_thread_equal() should just compare two unsigned longs on windows.

The issue with that is that uv_thread_start() returns uv_thread_t and you wouldn't be able to compare that with the return from uv_thread_self(). In fact, you would only be able to compare the return of uv_thread_self() with another value returned by the same function.

Per discussion with @piscisaureus , the viable options seem either a) to either extend the API to distinguish the thread object type the thread id type, or b) to implement the current API correctly on Windows, which would require being able to retrieve the creation handle for the current thread (using TLS). I am investigating further.

orangemocha commented 10 years ago

Better solution at https://github.com/joyent/libuv/pull/1570. Closing this one.