[Bug 50292] Process-local synchronization objects use private interfaces into the Unix library

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=50292

--- Comment #4 from Zebediah Figura <z.figura12 at 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.

-- 
Do not reply to this email, post in Bugzilla using the
above URL to reply.
You are receiving this mail because:
You are watching all bug changes.