From 774ee652a9c73c0bc3cdba26eba0c112f079cee4 Mon Sep 17 00:00:00 2001 From: Phil Vachon Date: Fri, 1 Aug 2025 16:20:52 -0400 Subject: [PATCH 1/5] Fix YUV conversion for systems with lots of CPUs The YUV CODEC test case exposed a bug where, for an image with a height less than the number of CPUs in a system, the block size in lines would end up being zero. This resulted in a divide by zero when parcelling up workloads for the thread pool. --- libfreerdp/codec/yuv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libfreerdp/codec/yuv.c b/libfreerdp/codec/yuv.c index debd29fe7..75d8a6a89 100644 --- a/libfreerdp/codec/yuv.c +++ b/libfreerdp/codec/yuv.c @@ -167,7 +167,8 @@ BOOL yuv_context_reset(YUV_CONTEXT* WINPR_RESTRICT context, UINT32 width, UINT32 context->width = width; context->height = height; - context->heightStep = (height / context->nthreads); + + context->heightStep = height > context->nthreads ? (height / context->nthreads) : 1; if (context->useThreads) { From e1db51383b07bc79d6d43a12fe304d54726f9b75 Mon Sep 17 00:00:00 2001 From: Phil Vachon Date: Fri, 1 Aug 2025 17:01:24 -0400 Subject: [PATCH 2/5] Use OS-provided thread ID primitives Truncating 64-bit thread IDs to 32-bits can result in collisions on systems with large numbers of CPUs. GetCurrentThreadId() does this, and there are fairly easy to reproduce collisions on systems with ~128 CPU cores. The `GetCurrentThreadId` API should wrap an OS-specific way of providing the current thread ID. Keep the XOR-folded pointer as a fallback for OSes which aren't explicitly supported. This fixes failures in TestSynchCritical that are readily reproducible on such systems.. Tested on FreeBSD 14.3 and various Linux variants on x86_64. --- winpr/libwinpr/thread/thread.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/winpr/libwinpr/thread/thread.c b/winpr/libwinpr/thread/thread.c index d9b04fc91..d6c546768 100644 --- a/winpr/libwinpr/thread/thread.c +++ b/winpr/libwinpr/thread/thread.c @@ -29,6 +29,13 @@ #include +#if defined(__MACOSX__) || defined(__FreeBSD__) +#include +#elif defined(__linux__) +#include + +#endif + #ifndef MIN #define MIN(x, y) (((x) < (y)) ? (x) : (y)) #endif @@ -911,11 +918,20 @@ HANDLE _GetCurrentThread(VOID) DWORD GetCurrentThreadId(VOID) { +#if defined(__FreeBSD__) || defined(__MACOSX__) + int tid = pthread_getthreadid_np(); + return tid; +#elif defined(__linux__) + pid_t tid = syscall(SYS_gettid); + return tid; +#else +#warning Using possibly broken GetCurrentThreadId pthread_t tid = pthread_self(); /* Since pthread_t can be 64-bits on some systems, take just the */ /* lower 32-bits of it for the thread ID returned by this function. */ uintptr_t ptid = WINPR_REINTERPRET_CAST(tid, pthread_t, uintptr_t); - return ptid & UINT32_MAX; + return (ptid & UINT32_MAX) ^ (ptid >> 32); +#endif } typedef struct From 91f928e0184cc50fde85e5b7a3ec9abca7e20772 Mon Sep 17 00:00:00 2001 From: Phil Vachon Date: Thu, 7 Aug 2025 10:07:28 -0400 Subject: [PATCH 3/5] Switch macOS to use fallback for thread ID macOS uses a 64-bit thread ID, so rather than truncate this in GetCurrentThreadId, use pthread_self's result and mix it with the MSBs of the pointer to reduce the probability of a collision. --- winpr/libwinpr/thread/thread.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/winpr/libwinpr/thread/thread.c b/winpr/libwinpr/thread/thread.c index d6c546768..e50efec58 100644 --- a/winpr/libwinpr/thread/thread.c +++ b/winpr/libwinpr/thread/thread.c @@ -29,11 +29,10 @@ #include -#if defined(__MACOSX__) || defined(__FreeBSD__) +#if defined(__FreeBSD__) #include #elif defined(__linux__) #include - #endif #ifndef MIN @@ -918,14 +917,13 @@ HANDLE _GetCurrentThread(VOID) DWORD GetCurrentThreadId(VOID) { -#if defined(__FreeBSD__) || defined(__MACOSX__) +#if defined(__FreeBSD__) int tid = pthread_getthreadid_np(); return tid; #elif defined(__linux__) pid_t tid = syscall(SYS_gettid); return tid; #else -#warning Using possibly broken GetCurrentThreadId pthread_t tid = pthread_self(); /* Since pthread_t can be 64-bits on some systems, take just the */ /* lower 32-bits of it for the thread ID returned by this function. */ From 9e217fc3ac813956d73474682ee0a7a972760d6a Mon Sep 17 00:00:00 2001 From: Phil Vachon Date: Sun, 10 Aug 2025 13:01:54 -0400 Subject: [PATCH 4/5] Explicitly cast away signs for thread IDs Neither FreeBSD nor Linux specify a negative value for a thread ID, so cast the sign away explicitly. --- winpr/libwinpr/thread/thread.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/winpr/libwinpr/thread/thread.c b/winpr/libwinpr/thread/thread.c index e50efec58..c0698e41c 100644 --- a/winpr/libwinpr/thread/thread.c +++ b/winpr/libwinpr/thread/thread.c @@ -918,10 +918,10 @@ HANDLE _GetCurrentThread(VOID) DWORD GetCurrentThreadId(VOID) { #if defined(__FreeBSD__) - int tid = pthread_getthreadid_np(); + int tid = WINPR_CXX_COMPAT_CAST(DWORD, pthread_getthreadid_np()); return tid; #elif defined(__linux__) - pid_t tid = syscall(SYS_gettid); + pid_t tid = WINPR_CXX_COMPAT_CAST(DWORD, syscall(SYS_gettid)); return tid; #else pthread_t tid = pthread_self(); From e620bf6c53784ec0e9450e00d94b76c8af73e15a Mon Sep 17 00:00:00 2001 From: Phil Vachon Date: Sun, 10 Aug 2025 15:09:29 -0400 Subject: [PATCH 5/5] Actually make the correct cast The cast works best if you don't imply another conversion to a temporary. Just return the value directly from the syscall/pthreads call, while casting to a DWORD. --- winpr/libwinpr/thread/thread.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/winpr/libwinpr/thread/thread.c b/winpr/libwinpr/thread/thread.c index c0698e41c..5323078f3 100644 --- a/winpr/libwinpr/thread/thread.c +++ b/winpr/libwinpr/thread/thread.c @@ -918,11 +918,9 @@ HANDLE _GetCurrentThread(VOID) DWORD GetCurrentThreadId(VOID) { #if defined(__FreeBSD__) - int tid = WINPR_CXX_COMPAT_CAST(DWORD, pthread_getthreadid_np()); - return tid; + return WINPR_CXX_COMPAT_CAST(DWORD, pthread_getthreadid_np()); #elif defined(__linux__) - pid_t tid = WINPR_CXX_COMPAT_CAST(DWORD, syscall(SYS_gettid)); - return tid; + return WINPR_CXX_COMPAT_CAST(DWORD, syscall(SYS_gettid)); #else pthread_t tid = pthread_self(); /* Since pthread_t can be 64-bits on some systems, take just the */