[PATCH v11 0/2] MR4372: kernel32: impl sync barrier

[quininer (＠quininer)]

This is my first time contributing wine and there are a lot of things I'm not sure about, so any comments are welcome. :)

--
  v11: kernel32: add simple sync barrier test
       kernel32: impl sync barrier

https://gitlab.winehq.org/wine/wine/-/merge_requests/4372

[PATCH v11 1/2] kernel32: impl sync barrier

[quininer]

From: quininer

---
 dlls/kernel32/kernel32.spec     |  3 ++
 dlls/kernelbase/kernelbase.spec |  6 +--
 dlls/kernelbase/sync.c          | 25 +++++++++++++
 dlls/ntdll/ntdll.spec           |  3 ++
 dlls/ntdll/sync.c               | 66 +++++++++++++++++++++++++++++++++
 include/synchapi.h              | 14 +++++++
 include/winnt.h                 |  8 ++++
 7 files changed, 122 insertions(+), 3 deletions(-)

diff --git a/dlls/kernel32/kernel32.spec b/dlls/kernel32/kernel32.spec
index 1deeb2a2ca5..081147d827d 100644
--- a/dlls/kernel32/kernel32.spec
+++ b/dlls/kernel32/kernel32.spec
@@ -375,6 +375,7 @@
 @ stdcall DisassociateCurrentThreadFromCallback(ptr) NTDLL.TpDisassociateCallback
 @ stdcall DiscardVirtualMemory(ptr long) kernelbase.DiscardVirtualMemory
 @ stdcall DeleteTimerQueue(long)
+@ stdcall -import DeleteSynchronizationBarrier(ptr)
 @ stdcall -import DeleteTimerQueueEx(long long)
 @ stdcall -import DeleteTimerQueueTimer(long long long)
 @ stdcall -arch=win64 DeleteUmsCompletionList(ptr)
@@ -398,6 +399,7 @@
 @ stdcall EndUpdateResourceA(long long)
 @ stdcall EndUpdateResourceW(long long)
 @ stdcall EnterCriticalSection(ptr) NTDLL.RtlEnterCriticalSection
+@ stdcall -import EnterSynchronizationBarrier(ptr long)
 @ stdcall EnumCalendarInfoA(ptr long long long)
 @ stdcall EnumCalendarInfoExA(ptr long long long)
 @ stdcall -import EnumCalendarInfoExEx(ptr wstr long wstr long long)
@@ -970,6 +972,7 @@
 @ stdcall -import InitializeProcThreadAttributeList(ptr long long ptr)
 @ stdcall InitializeSListHead(ptr) NTDLL.RtlInitializeSListHead
 @ stdcall InitializeSRWLock(ptr) NTDLL.RtlInitializeSRWLock
+@ stdcall -import InitializeSynchronizationBarrier(ptr long long)
 @ stdcall -arch=i386 InterlockedCompareExchange (ptr long long)
 @ stdcall -arch=i386 -ret64 InterlockedCompareExchange64(ptr int64 int64) NTDLL.RtlInterlockedCompareExchange64
 @ stdcall -arch=i386 InterlockedDecrement(ptr)
diff --git a/dlls/kernelbase/kernelbase.spec b/dlls/kernelbase/kernelbase.spec
index e85f2b67d74..c3c6d8ed0dd 100644
--- a/dlls/kernelbase/kernelbase.spec
+++ b/dlls/kernelbase/kernelbase.spec
@@ -260,7 +260,7 @@
 # @ stub DeleteStateAtomValue
 # @ stub DeleteStateContainer
 # @ stub DeleteStateContainerValue
-# @ stub DeleteSynchronizationBarrier
+@ stdcall DeleteSynchronizationBarrier(ptr)
 @ stdcall DeleteTimerQueueEx(long long)
 @ stdcall DeleteTimerQueueTimer(long long long)
 @ stdcall DeleteVolumeMountPointW(wstr)
@@ -293,7 +293,7 @@
 @ stdcall EncodeSystemPointer(ptr) ntdll.RtlEncodeSystemPointer
 # @ stub EnterCriticalPolicySectionInternal
 @ stdcall EnterCriticalSection(ptr) ntdll.RtlEnterCriticalSection
-# @ stub EnterSynchronizationBarrier
+@ stdcall EnterSynchronizationBarrier(ptr long)
 @ stdcall EnumCalendarInfoExEx(ptr wstr long wstr long long)
 @ stdcall EnumCalendarInfoExW(ptr long long long)
 @ stdcall EnumCalendarInfoW(ptr long long long)
@@ -843,7 +843,7 @@
 @ stdcall InitializeSRWLock(ptr) ntdll.RtlInitializeSRWLock
 @ stdcall InitializeSecurityDescriptor(ptr long)
 @ stdcall InitializeSid(ptr ptr long)
-# @ stub InitializeSynchronizationBarrier
+@ stdcall InitializeSynchronizationBarrier(ptr long long)
 # @ stub InstallELAMCertificateInfo
 @ stdcall -arch=i386 InterlockedCompareExchange(ptr long long)
 @ stdcall -arch=i386 -ret64 InterlockedCompareExchange64(ptr int64 int64) ntdll.RtlInterlockedCompareExchange64
diff --git a/dlls/kernelbase/sync.c b/dlls/kernelbase/sync.c
index b2086207caa..a7943778891 100644
--- a/dlls/kernelbase/sync.c
+++ b/dlls/kernelbase/sync.c
@@ -1684,6 +1684,31 @@ BOOL WINAPI DECLSPEC_HOTPATCH InitOnceExecuteOnce( INIT_ONCE *once, PINIT_ONCE_F
     return !RtlRunOnceExecuteOnce( once, (PRTL_RUN_ONCE_INIT_FN)func, param, context );
 }

+/***********************************************************************
+ *           InitializeSynchronizationBarrier    (kernelbase.@)
+ */
+BOOL WINAPI DECLSPEC_HOTPATCH InitializeSynchronizationBarrier( LPSYNCHRONIZATION_BARRIER barrier, LONG total_threads,
+                                                                LONG spin_count )
+{
+    return set_ntstatus( RtlInitBarrier( barrier, total_threads, spin_count ));
+}
+
+/***********************************************************************
+ *           EnterSynchronizationBarrier    (kernelbase.@)
+ */
+BOOL WINAPI DECLSPEC_HOTPATCH EnterSynchronizationBarrier( LPSYNCHRONIZATION_BARRIER barrier, DWORD flags )
+{
+    return RtlBarrier(barrier, flags);
+}
+
+/***********************************************************************
+ *           DeleteSynchronizationBarrier    (kernelbase.@)
+ */
+BOOL WINAPI DECLSPEC_HOTPATCH DeleteSynchronizationBarrier( LPSYNCHRONIZATION_BARRIER barrier )
+{
+    return set_ntstatus( RtlDeleteBarrier( barrier ));
+}
+
 #ifdef __i386__

/***********************************************************************
diff --git a/dlls/ntdll/ntdll.spec b/dlls/ntdll/ntdll.spec
index c09157a369b..f9eac0744ef 100644
--- a/dlls/ntdll/ntdll.spec
+++ b/dlls/ntdll/ntdll.spec
@@ -514,6 +514,7 @@
 @ stdcall RtlAreBitsSet(ptr long long)
 # @ stub RtlAssert2
 @ stdcall RtlAssert(ptr ptr long str)
+@ stdcall RtlBarrier(ptr long)
 # @ stub RtlCancelTimer
 @ stdcall -norelay RtlCaptureContext(ptr)
 @ stdcall RtlCaptureStackBackTrace(long long ptr ptr)
@@ -598,6 +599,7 @@
 @ stub RtlDelete
 @ stdcall RtlDeleteAce(ptr long)
 @ stdcall RtlDeleteAtomFromAtomTable(ptr long)
+@ stdcall RtlDeleteBarrier(ptr)
 @ stdcall RtlDeleteCriticalSection(ptr)
 @ stdcall -arch=!i386 RtlDeleteGrowableFunctionTable(ptr)
 @ stub RtlDeleteElementGenericTable
@@ -777,6 +779,7 @@
 @ stdcall RtlImpersonateSelf(long)
 @ stdcall RtlInitAnsiString(ptr str)
 @ stdcall RtlInitAnsiStringEx(ptr str)
+@ stdcall RtlInitBarrier(ptr long long)
 @ stdcall RtlInitCodePageTable(ptr ptr)
 # @ stub RtlInitMemoryStream
 @ stdcall RtlInitNlsTables(ptr ptr ptr ptr)
diff --git a/dlls/ntdll/sync.c b/dlls/ntdll/sync.c
index bb5dcbb66e6..8ef5e1ef56e 100644
--- a/dlls/ntdll/sync.c
+++ b/dlls/ntdll/sync.c
@@ -1000,3 +1000,69 @@ void WINAPI RtlWakeAddressSingle( const void *addr )

if (tid) NtAlertThreadByThreadId( (HANDLE)(DWORD_PTR)tid );
 }
+
+
+typedef struct {
+    RTL_SRWLOCK lock;
+    RTL_CONDITION_VARIABLE cond;
+    ULONG count;
+} barrier_entry;
+C_ASSERT( sizeof(barrier_entry) <= sizeof(RTL_BARRIER) );
+
+/***********************************************************************
+ *           RtlInitBarrier     (NTDLL.@)
+ */
+NTSTATUS WINAPI RtlInitBarrier( PRTL_BARRIER Barrier, ULONG TotalThreads, ULONG SpinCount )
+{
+    if(!Barrier)
+        return STATUS_INVALID_PARAMETER;
+
+    barrier_entry *barrier = (barrier_entry*)Barrier;
+
+    RtlInitializeSRWLock(&barrier->lock);
+    RtlInitializeConditionVariable(&barrier->cond);
+    barrier->count = TotalThreads;
+
+    return STATUS_SUCCESS;
+}
+
+/***********************************************************************
+ *           RtlBarrier         (NTDLL.@)
+ */
+BOOLEAN WINAPI RtlBarrier( PRTL_BARRIER Barrier, ULONG Flags )
+{
+    barrier_entry *barrier = (barrier_entry*)Barrier;
+    BOOL wait = TRUE;
+    BOOL hint = FALSE;
+
+    RtlAcquireSRWLockExclusive(&barrier->lock);
+
+    if (barrier->count > 0) {
+        barrier->count -= 1;
+    }
+
+    if (barrier->count == 0) {
+        RtlWakeAllConditionVariable(&barrier->cond);
+        wait = FALSE;
+        hint = TRUE;
+    }
+
+    while (wait) {
+        RtlSleepConditionVariableSRW(&barrier->cond, &barrier->lock, NULL, 0);
+        wait = barrier->count > 0;
+    }
+
+    RtlReleaseSRWLockExclusive(&barrier->lock);
+
+    return hint;
+}
+
+/***********************************************************************
+ *           RtlDeleteBarrier   (NTDLL.@)
+ */
+NTSTATUS WINAPI RtlDeleteBarrier( PRTL_BARRIER Barrier )
+{
+    if(!Barrier)
+        return STATUS_INVALID_PARAMETER;
+    return STATUS_SUCCESS;
+}
diff --git a/include/synchapi.h b/include/synchapi.h
index 0b3dcb8ff74..2c33907da6b 100644
--- a/include/synchapi.h
+++ b/include/synchapi.h
@@ -19,6 +19,8 @@
 #ifndef _SYNCHAPI_H
 #define _SYNCHAPI_H

+#include <winbase.h>
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -27,6 +29,18 @@ BOOL WINAPI WaitOnAddress(volatile void*, void*, SIZE_T, DWORD);
 void WINAPI WakeByAddressAll(void*);
 void WINAPI WakeByAddressSingle(void*);

+typedef RTL_BARRIER SYNCHRONIZATION_BARRIER;
+typedef PRTL_BARRIER PSYNCHRONIZATION_BARRIER;
+typedef PRTL_BARRIER LPSYNCHRONIZATION_BARRIER;
+
+#define SYNCHRONIZATION_BARRIER_FLAGS_SPIN_ONLY 0x01
+#define SYNCHRONIZATION_BARRIER_FLAGS_BLOCK_ONLY 0x02
+#define SYNCHRONIZATION_BARRIER_FLAGS_NO_DELETE 0x04
+
+BOOL WINAPI InitializeSynchronizationBarrier(LPSYNCHRONIZATION_BARRIER lpBarrier, LONG lTotalThreads, LONG lSpinCount);
+BOOL WINAPI EnterSynchronizationBarrier(LPSYNCHRONIZATION_BARRIER lpBarrier, DWORD dwFlags);
+BOOL WINAPI DeleteSynchronizationBarrier(LPSYNCHRONIZATION_BARRIER lpBarrier);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/include/winnt.h b/include/winnt.h
index d1c82084f9f..7983b1eea4d 100644
--- a/include/winnt.h
+++ b/include/winnt.h
@@ -6097,6 +6097,14 @@ NTSYSAPI DWORD WINAPI RtlRunOnceExecuteOnce(PRTL_RUN_ONCE,PRTL_RUN_ONCE_INIT_FN,
 NTSYSAPI DWORD WINAPI RtlRunOnceBeginInitialize(PRTL_RUN_ONCE, DWORD, PVOID*);
 NTSYSAPI DWORD WINAPI RtlRunOnceComplete(PRTL_RUN_ONCE, DWORD, PVOID);

+typedef struct _RTL_BARRIER {
+    DWORD Reserved1;
+    DWORD Reserved2;
+    ULONG_PTR Reserved3[2];
+    DWORD Reserved4;
+    DWORD Reserved5;
+} RTL_BARRIER, *PRTL_BARRIER;
+
 #include <pshpack8.h>
 typedef struct _IO_COUNTERS {
     ULONGLONG DECLSPEC_ALIGN(8) ReadOperationCount;

[PATCH v11 2/2] kernel32: add simple sync barrier test

[quininer]

From: quininer

---
 dlls/kernel32/tests/sync.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/dlls/kernel32/tests/sync.c b/dlls/kernel32/tests/sync.c
index 56a9d6e4859..c15cfd14c0b 100644
--- a/dlls/kernel32/tests/sync.c
+++ b/dlls/kernel32/tests/sync.c
@@ -2841,6 +2841,42 @@ static void test_QueueUserAPC(void)
     ok(apc_count == 1, "APC count %u\n", apc_count);
 }

+static DWORD WINAPI barrier_worker(LPVOID b) {
+    SYNCHRONIZATION_BARRIER *barrier = (SYNCHRONIZATION_BARRIER*)b;
+    EnterSynchronizationBarrier(barrier, 0);
+    return 0;
+}
+
+static void test_barrier(void)
+{
+    SYNCHRONIZATION_BARRIER barrier;
+    DWORD dummy;
+    DWORD r;
+
+    r = InitializeSynchronizationBarrier(&barrier, 3, 0);
+    ok( r == TRUE, "init sync barrier failed\n");
+
+    CreateThread(NULL, 0, barrier_worker, &barrier, 0, &dummy);
+    CreateThread(NULL, 0, barrier_worker, &barrier, 0, &dummy);
+
+    EnterSynchronizationBarrier(&barrier, 0);
+    r = DeleteSynchronizationBarrier(&barrier);
+    ok( r == TRUE, "delete sync barrier failed\n");
+
+    // reuse barrier object
+    r = InitializeSynchronizationBarrier(&barrier, 5, 0);
+    ok( r == TRUE, "init sync barrier failed\n");
+
+    CreateThread(NULL, 0, barrier_worker, &barrier, 0, &dummy);
+    CreateThread(NULL, 0, barrier_worker, &barrier, 0, &dummy);
+    CreateThread(NULL, 0, barrier_worker, &barrier, 0, &dummy);
+    CreateThread(NULL, 0, barrier_worker, &barrier, 0, &dummy);
+
+    EnterSynchronizationBarrier(&barrier, 0);
+    r = DeleteSynchronizationBarrier(&barrier);
+    ok( r == TRUE, "delete sync barrier failed\n");
+}
+
 START_TEST(sync)
 {
     char **argv;
@@ -2912,4 +2948,5 @@ START_TEST(sync)
     test_alertable_wait();
     test_apc_deadlock();
     test_crit_section();
+    test_barrier();
 }

[PATCH v11 2/2] kernel32: add simple sync barrier test

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=139901

Your paranoid android.

=== w8 (32 bit report) ===

kernel32:
sync.c:2864: Test failed: delete sync barrier failed
sync.c:2877: Test failed: delete sync barrier failed

=== w8adm (32 bit report) ===

kernel32:
sync.c:2864: Test failed: delete sync barrier failed
sync.c:2877: Test failed: delete sync barrier failed

=== w864 (32 bit report) ===

kernel32:
sync.c:2864: Test failed: delete sync barrier failed
sync.c:2877: Test failed: delete sync barrier failed

=== w1064v1507 (32 bit report) ===

kernel32:
sync.c:2864: Test failed: delete sync barrier failed
sync.c:2877: Test failed: delete sync barrier failed

=== w864 (64 bit report) ===

kernel32:
sync.c:2864: Test failed: delete sync barrier failed
sync.c:2877: Test failed: delete sync barrier failed

=== w1064v1507 (64 bit report) ===

kernel32:
sync.c:2864: Test failed: delete sync barrier failed
sync.c:2877: Test failed: delete sync barrier failed

=== debian11 (build log) ===

/home/winetest/tools/testbot/var/wine-win32/../wine/dlls/kernelbase/sync.c:1693: undefined reference to `RtlInitBarrier'
/home/winetest/tools/testbot/var/wine-win32/../wine/dlls/kernelbase/sync.c:1701: undefined reference to `RtlBarrier'
/home/winetest/tools/testbot/var/wine-win32/../wine/dlls/kernelbase/sync.c:1709: undefined reference to `RtlDeleteBarrier'
collect2: error: ld returned 1 exit status
Task: The win32 Wine build failed

=== debian11b (build log) ===

/home/winetest/tools/testbot/var/wine-wow32/../wine/dlls/kernelbase/sync.c:1693: undefined reference to `RtlInitBarrier'
/home/winetest/tools/testbot/var/wine-wow32/../wine/dlls/kernelbase/sync.c:1701: undefined reference to `RtlBarrier'
/home/winetest/tools/testbot/var/wine-wow32/../wine/dlls/kernelbase/sync.c:1709: undefined reference to `RtlDeleteBarrier'
collect2: error: ld returned 1 exit status
Task: The wow32 Wine build failed

[Etaash Mathamsetty (＠etaash.mathamsetty)]

Etaash Mathamsetty (@etaash.mathamsetty) commented about dlls/kernel32/tests/sync.c:
> - SYNCHRONIZATION_BARRIER barrier;
> - DWORD dummy;
> - DWORD r;
> -
> - r = InitializeSynchronizationBarrier(&barrier, 3, 0);
> - ok( r == TRUE, "init sync barrier failed\n");
> -
> - CreateThread(NULL, 0, barrier_worker, &barrier, 0, &dummy);
> - CreateThread(NULL, 0, barrier_worker, &barrier, 0, &dummy);
> -
> - EnterSynchronizationBarrier(&barrier, 0);
> - r = DeleteSynchronizationBarrier(&barrier);
> - ok( r == TRUE, "delete sync barrier failed\n");
> -
> - // reuse barrier object
> - r = InitializeSynchronizationBarrier(&barrier, 5, 0);

By reusing I meant reusing without reinitializing    
btw you have to make comments like /* */ rather than // when contributing to wine

[quininer (＠quininer)]

On Wed Nov 15 22:29:00 2023 +0000, Etaash Mathamsetty wrote:
> By reusing I meant reusing without reinitializing    
> btw you have to make comments like /* */ rather than // when
> contributing to wine

Ah, I subconsciously never thought of it as "reuse". I'll try to write a more useful test.