This allows *BSDs to also have a fast path similar to futexes for thread-ID alerts. Also a kevent with `EV_CLEAR` and `NOTE_TRIGGER` maps perfectly to the thread alertable state, fixing the issue of the current mach-semaphore implementation not correctly waiting in the test case below: ``` NtAlertThreadByThreadId((HANDLE)GetCurrentThreadId()); NtAlertThreadByThreadId((HANDLE)GetCurrentThreadId()); NtWaitForAlertByThreadId(NULL, NULL); NtWaitForAlertByThreadId(NULL, NULL); ``` I took the liberty to remove this mach semaphore implementation, since all versions of OSX have supported kqueue().
-- v6: ntdll: Implement thread-ID alerts using kqueue/kevent.
From: Marc-Aurel Zent marc_aurel@me.com
--- dlls/ntdll/unix/sync.c | 96 ++++++++++++++++++++++++++++-------------- 1 file changed, 65 insertions(+), 31 deletions(-)
diff --git a/dlls/ntdll/unix/sync.c b/dlls/ntdll/unix/sync.c index d1e4e5ee111..4cb8bb581ee 100644 --- a/dlls/ntdll/unix/sync.c +++ b/dlls/ntdll/unix/sync.c @@ -49,11 +49,11 @@ #include <stdlib.h> #include <time.h> #ifdef __APPLE__ -# include <mach/mach.h> -# include <mach/task.h> -# include <mach/semaphore.h> # include <mach/mach_time.h> #endif +#ifdef HAVE_KQUEUE +# include <sys/event.h> +#endif
#include "ntstatus.h" #define WIN32_NO_STATUS @@ -2354,8 +2354,8 @@ NTSTATUS WINAPI NtQueryInformationAtom( RTL_ATOM atom, ATOM_INFORMATION_CLASS cl
union tid_alert_entry { -#ifdef __APPLE__ - semaphore_t sem; +#ifdef HAVE_KQUEUE + int kq; #else HANDLE event; #ifdef __linux__ @@ -2396,15 +2396,35 @@ static union tid_alert_entry *get_tid_alert_entry( HANDLE tid )
entry = &tid_alert_blocks[block_idx][idx % TID_ALERT_BLOCK_SIZE];
-#ifdef __APPLE__ - if (!entry->sem) +#ifdef HAVE_KQUEUE + if (!entry->kq) { - semaphore_t sem; + int kq = kqueue(); + static const struct kevent init_event = + { + .ident = 1, + .filter = EVFILT_USER, + .flags = EV_ADD | EV_CLEAR, + .fflags = 0, + .data = 0, + .udata = NULL + }; + + if (kq == -1) + { + ERR( "kqueue failed with error: %d (%s)\n", errno, strerror( errno ) ); + return NULL; + }
- if (semaphore_create( mach_task_self(), &sem, SYNC_POLICY_FIFO, 0 )) + if (kevent( kq, &init_event, 1, NULL, 0, NULL) == -1) + { + ERR( "kevent creation failed with error: %d (%s)\n", errno, strerror( errno ) ); + close( kq ); return NULL; - if (InterlockedCompareExchange( (LONG *)&entry->sem, sem, 0 )) - semaphore_destroy( mach_task_self(), sem ); + } + + if (InterlockedCompareExchange( (LONG *)&entry->kq, kq, 0 )) + close( kq ); } #else #ifdef __linux__ @@ -2438,9 +2458,21 @@ NTSTATUS WINAPI NtAlertThreadByThreadId( HANDLE tid )
if (!entry) return STATUS_INVALID_CID;
-#ifdef __APPLE__ - semaphore_signal( entry->sem ); - return STATUS_SUCCESS; +#ifdef HAVE_KQUEUE + { + static const struct kevent signal_event = + { + .ident = 1, + .filter = EVFILT_USER, + .flags = 0, + .fflags = NOTE_TRIGGER, + .data = 0, + .udata = NULL + }; + + kevent( entry->kq, &signal_event, 1, NULL, 0, NULL ); + return STATUS_SUCCESS; + } #else #ifdef __linux__ if (use_futexes()) @@ -2457,7 +2489,7 @@ NTSTATUS WINAPI NtAlertThreadByThreadId( HANDLE tid ) }
-#if defined(__linux__) || defined(__APPLE__) +#if defined(__linux__) || defined(HAVE_KQUEUE) static LONGLONG get_absolute_timeout( const LARGE_INTEGER *timeout ) { LARGE_INTEGER now; @@ -2480,7 +2512,7 @@ static LONGLONG update_timeout( ULONGLONG end ) #endif
-#ifdef __APPLE__ +#ifdef HAVE_KQUEUE
/*********************************************************************** * NtWaitForAlertByThreadId (NTDLL.@) @@ -2488,14 +2520,14 @@ static LONGLONG update_timeout( ULONGLONG end ) NTSTATUS WINAPI NtWaitForAlertByThreadId( const void *address, const LARGE_INTEGER *timeout ) { union tid_alert_entry *entry = get_tid_alert_entry( NtCurrentTeb()->ClientId.UniqueThread ); - semaphore_t sem; ULONGLONG end; - kern_return_t ret; + int ret; + struct timespec timespec; + struct kevent wait_event;
TRACE( "%p %s\n", address, debugstr_timeout( timeout ) );
if (!entry) return STATUS_INVALID_CID; - sem = entry->sem;
if (timeout) { @@ -2505,27 +2537,29 @@ NTSTATUS WINAPI NtWaitForAlertByThreadId( const void *address, const LARGE_INTEG end = get_absolute_timeout( timeout ); }
- for (;;) + do { if (timeout) { LONGLONG timeleft = update_timeout( end ); - mach_timespec_t timespec;
timespec.tv_sec = timeleft / (ULONGLONG)TICKSPERSEC; timespec.tv_nsec = (timeleft % TICKSPERSEC) * 100; - ret = semaphore_timedwait( sem, timespec ); + if (timespec.tv_sec > 0x7FFFFFFF) timeout = NULL; } - else - ret = semaphore_wait( sem );
- switch (ret) - { - case KERN_SUCCESS: return STATUS_ALERTED; - case KERN_ABORTED: continue; - case KERN_OPERATION_TIMED_OUT: return STATUS_TIMEOUT; - default: return STATUS_INVALID_HANDLE; - } + ret = kevent( entry->kq, NULL, 0, &wait_event, 1, timeout ? ×pec : NULL ); + } while (ret == -1 && errno == EINTR); + + switch (ret) + { + case 1: + return STATUS_ALERTED; + case 0: + return STATUS_TIMEOUT; + default: + ERR( "kevent failed with error: %d (%s)\n", errno, strerror( errno ) ); + return STATUS_INVALID_HANDLE; } }
I removed the usleep implementation and will see try to make something work with `dlsym()` there. The points above should be addressed now, thanks for looking over it.
On Fri Oct 20 15:26:12 2023 +0000, Marc-Aurel Zent wrote:
I removed the ulock implementation and will try to make something work with `dlsym()` there. The points above should be addressed now, thanks for looking over it.
Great thanks, looks good to me.
-
Brendan Shanks (＠bshanks) -
Marc-Aurel Zent
-
Marc-Aurel Zent (＠marzent)