https://bugs.winehq.org/show_bug.cgi?id=50292
--- Comment #4 from Zebediah Figura z.figura12@gmail.com --- (In reply to Rémi Bernon from comment #1)
Hi, I had a quick look at the patch series and here's a few nitpicks that I've found:
In 0004-ntdll-Implement-NtAlertThreadByThreadId-and-NtWaitFo.patch:
if (teb->ClientId.UniqueThread == tid){pthread_rwlock_unlock( &teb_list_lock );NtSetEvent( thread_data->tid_alert_event, NULL );return STATUS_SUCCESS;}I think there's a race condition here, were the thread could potentially be interrupted after the TEB lock is released, but before the event is set.
The other thread that thread_data refers to may then terminate, the NtSetEvent call may set an non-existing event, or worse if the TEB is reused, and the new thread waiting itself, wake a wrong thread.
It's probably unlikely to happen but from a correctness point of view I think it's wrong.
I think you're right, this is a race and there's not much of a penalty to fixing it.
(In reply to Rémi Bernon from comment #3)
(In reply to Rémi Bernon from comment #2)
If FUTEX_WAIT_BITSET is available, I think you can use it to wait with an absolute timeout, which could save calls to NtQuerySystemTime.
Or maybe it's on purpose because of CLOCK_MONOTONIC NTP adjustments? In which case please just ignore my comment.
Probably, yes, we'd need a RAW flag. Note though that the timeout is always relative when called through kernel32, so it doesn't help us much anyway.