This is my first time contributing wine and there are a lot of things I'm not sure about, so any comments are welcome. :)
-- v8: kernel32: impl sync barrier
From: quininer quininer@live.com
--- dlls/kernel32/kernel32.spec | 3 ++ dlls/kernel32/tests/sync.c | 24 +++++++++++++ dlls/kernelbase/kernelbase.spec | 6 ++-- dlls/ntdll/ntdll.spec | 3 ++ dlls/ntdll/sync.c | 61 +++++++++++++++++++++++++++++++++ include/synchapi.h | 14 ++++++++ include/winnt.h | 8 +++++ 7 files changed, 116 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/kernel32/tests/sync.c b/dlls/kernel32/tests/sync.c index 56a9d6e4859..b45916711ba 100644 --- a/dlls/kernel32/tests/sync.c +++ b/dlls/kernel32/tests/sync.c @@ -2841,6 +2841,29 @@ 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"); +} + START_TEST(sync) { char **argv; @@ -2912,4 +2935,5 @@ START_TEST(sync) test_alertable_wait(); test_apc_deadlock(); test_crit_section(); + test_barrier(); } diff --git a/dlls/kernelbase/kernelbase.spec b/dlls/kernelbase/kernelbase.spec index e85f2b67d74..c908e5cae1a 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) ntdll.RtlDeleteBarrier @ 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) ntdll.RtlBarrier @ 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) ntdll.RtlInitBarrier # @ stub InstallELAMCertificateInfo @ stdcall -arch=i386 InterlockedCompareExchange(ptr long long) @ stdcall -arch=i386 -ret64 InterlockedCompareExchange64(ptr int64 int64) ntdll.RtlInterlockedCompareExchange64 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..eef1ca6c195 100644 --- a/dlls/ntdll/sync.c +++ b/dlls/ntdll/sync.c @@ -1000,3 +1000,64 @@ 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 ) +{ + 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 ) +{ + 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;
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 tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details:
The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=139789
Your paranoid android.
=== w8 (32 bit report) ===
kernel32: sync.c:2864: Test failed: delete sync barrier failed
=== w8adm (32 bit report) ===
kernel32: sync.c:2864: Test failed: delete sync barrier failed
=== w864 (32 bit report) ===
kernel32: sync.c:2864: Test failed: delete sync barrier failed
=== w1064v1507 (32 bit report) ===
kernel32: sync.c:2864: Test failed: delete sync barrier failed
=== w864 (64 bit report) ===
kernel32: sync.c:2864: Test failed: delete sync barrier failed
=== w1064v1507 (64 bit report) ===
kernel32: sync.c:2864: Test failed: delete sync barrier failed
=== debian11 (32 bit report) ===
kernel32: sync.c:2857: Test failed: init sync barrier failed sync.c:2864: Test failed: delete sync barrier failed
=== debian11 (32 bit ar:MA report) ===
kernel32: sync.c:2857: Test failed: init sync barrier failed sync.c:2864: Test failed: delete sync barrier failed
=== debian11 (32 bit de report) ===
kernel32: sync.c:2857: Test failed: init sync barrier failed sync.c:2864: Test failed: delete sync barrier failed
=== debian11 (32 bit fr report) ===
kernel32: sync.c:2857: Test failed: init sync barrier failed sync.c:2864: Test failed: delete sync barrier failed
=== debian11 (32 bit he:IL report) ===
kernel32: sync.c:2857: Test failed: init sync barrier failed sync.c:2864: Test failed: delete sync barrier failed
=== debian11 (32 bit hi:IN report) ===
kernel32: sync.c:2857: Test failed: init sync barrier failed sync.c:2864: Test failed: delete sync barrier failed
=== debian11 (32 bit ja:JP report) ===
kernel32: sync.c:2857: Test failed: init sync barrier failed sync.c:2864: Test failed: delete sync barrier failed
=== debian11 (32 bit zh:CN report) ===
kernel32: sync.c:2857: Test failed: init sync barrier failed sync.c:2864: Test failed: delete sync barrier failed
=== debian11b (32 bit WoW report) ===
kernel32: sync.c:2857: Test failed: init sync barrier failed sync.c:2864: Test failed: delete sync barrier failed
=== debian11b (64 bit WoW report) ===
kernel32: sync.c:2857: Test failed: init sync barrier failed sync.c:2864: Test failed: delete sync barrier failed
Based on the docs https://learn.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-ini... and https://learn.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-ent..., i think that `SpinCount` and `Flags` do not affect behavior in this implement and can be safely ignored.
The return value of `EnterSynchronizationBarrier` does not an error, but rather whether it was the last barrier invoked.
I'm not familiar with how `NTSTATUS` should be handled here, do i need to forward it manually in `kernelbase`?
Barrier can only be used once.
I remember that barriers could be used multiple times if a certain flag was passed in.
Are there any applications that use this api?
@etaash.mathamsetty I don't see this behavior in docs. There is a `SYNCHRONIZATION_BARRIER_FLAGS_NO_DELETE`, but this is just an optimization flag and can be ignore.
I'm trying to get [bakin engine](https://rpgbakin.com/en) to run on wine, that's the first problem.
On Mon Nov 13 20:07:09 2023 +0000, quininer wrote:
@etaash.mathamsetty I don't see this behavior in docs. There is a `SYNCHRONIZATION_BARRIER_FLAGS_NO_DELETE`, but this is just an optimization flag and can be ignore. I'm trying to get [bakin engine](https://rpgbakin.com/en) to run on wine, that's the first problem.
that link doesn't seem to work, I also have made an implementation of this API around 1 year ago, I can fix it up to go through ntdll and send it here if you want
On Mon Nov 13 20:07:34 2023 +0000, Etaash Mathamsetty wrote:
that link doesn't seem to work, I also have made an implementation of this API around 1 year ago, I can fix it up to go through ntdll and send it here if you want
Of course, thank you!
On Tue Nov 14 13:20:05 2023 +0000, quininer wrote:
Of course, thank you!
this is probably not even close to accurate, but it seems to prevent the app from crashing: https://gitlab.winehq.org/etaash.mathamsetty/wine/-/commit/ad47b2c09a13cf890...
On Tue Nov 14 23:41:37 2023 +0000, Etaash Mathamsetty wrote:
this is probably not even close to accurate, but it seems to prevent the app from crashing: https://gitlab.winehq.org/etaash.mathamsetty/wine/-/commit/2e4fbc7221354229a... I also wrote this implementation when I was new to wine lol
You seem to have implement only spin, whereas I have implement only block. :D
I think it is sufficient to implement only block, and spin parameter is only a performance optimization for less competitive situations, and in many cases has a negative impact.
-
Etaash Mathamsetty (＠etaash.mathamsetty) -
Marvin -
quininer
-
quininer (＠quininer)