diff --git a/CMakeLists.txt b/CMakeLists.txt index 50cae49..39db2ea 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -484,6 +484,7 @@ if (WIBO_ENABLE_FIXTURE_TESTS) wibo_add_fixture_bin(NAME test_threading SOURCES test/test_threading.c) wibo_add_fixture_bin(NAME test_tls SOURCES test/test_tls.c) wibo_add_fixture_bin(NAME test_tls_reloc SOURCES test/test_tls_reloc.c) + wibo_add_fixture_bin(NAME test_fls SOURCES test/test_fls.c) wibo_add_fixture_bin(NAME test_handleapi SOURCES test/test_handleapi.c) wibo_add_fixture_bin(NAME test_findfile SOURCES test/test_findfile.c) wibo_add_fixture_bin(NAME test_getfileattributesex SOURCES test/test_getfileattributesex.c) diff --git a/dll/kernel32/fibersapi.cpp b/dll/kernel32/fibersapi.cpp index c834bd6..6b1b778 100644 --- a/dll/kernel32/fibersapi.cpp +++ b/dll/kernel32/fibersapi.cpp @@ -5,11 +5,30 @@ #include "errors.h" #include "internal.h" +#include +#include + namespace { constexpr DWORD kMaxFlsValues = 0x100; + +struct FlsSlot { + DWORD generation = 0; + LPVOID value = nullptr; +}; + +std::mutex g_flsMutex; bool g_flsValuesUsed[kMaxFlsValues] = {false}; -LPVOID g_flsValues[kMaxFlsValues] = {nullptr}; +DWORD g_flsGenerations[kMaxFlsValues] = {0}; +thread_local std::array t_flsValues; + +void resetThreadFlsSlot(DWORD index) { + FlsSlot &slot = t_flsValues[index]; + if (slot.generation != g_flsGenerations[index]) { + slot.generation = g_flsGenerations[index]; + slot.value = nullptr; + } +} } // namespace @@ -19,10 +38,12 @@ DWORD WINAPI FlsAlloc(PFLS_CALLBACK_FUNCTION lpCallback) { HOST_CONTEXT_GUARD(); DEBUG_LOG("FlsAlloc(%p)", lpCallback); // If the function succeeds, the return value is an FLS index initialized to zero. + std::lock_guard lk(g_flsMutex); for (DWORD i = 0; i < kMaxFlsValues; i++) { if (g_flsValuesUsed[i] == false) { g_flsValuesUsed[i] = true; - g_flsValues[i] = nullptr; + g_flsGenerations[i]++; + resetThreadFlsSlot(i); DEBUG_LOG(" -> %d\n", i); return i; } @@ -35,6 +56,7 @@ DWORD WINAPI FlsAlloc(PFLS_CALLBACK_FUNCTION lpCallback) { BOOL WINAPI FlsFree(DWORD dwFlsIndex) { HOST_CONTEXT_GUARD(); DEBUG_LOG("FlsFree(%u)\n", dwFlsIndex); + std::lock_guard lk(g_flsMutex); if (dwFlsIndex < kMaxFlsValues && g_flsValuesUsed[dwFlsIndex]) { g_flsValuesUsed[dwFlsIndex] = false; return TRUE; @@ -48,8 +70,10 @@ PVOID WINAPI FlsGetValue(DWORD dwFlsIndex) { HOST_CONTEXT_GUARD(); VERBOSE_LOG("FlsGetValue(%u)\n", dwFlsIndex); PVOID result = nullptr; + std::lock_guard lk(g_flsMutex); if (dwFlsIndex < kMaxFlsValues && g_flsValuesUsed[dwFlsIndex]) { - result = g_flsValues[dwFlsIndex]; + resetThreadFlsSlot(dwFlsIndex); + result = t_flsValues[dwFlsIndex].value; // See https://learn.microsoft.com/en-us/windows/win32/api/fibersapi/nf-fibersapi-flsgetvalue setLastError(ERROR_SUCCESS); } else { @@ -62,8 +86,10 @@ PVOID WINAPI FlsGetValue(DWORD dwFlsIndex) { BOOL WINAPI FlsSetValue(DWORD dwFlsIndex, PVOID lpFlsData) { HOST_CONTEXT_GUARD(); VERBOSE_LOG("FlsSetValue(%u, %p)\n", dwFlsIndex, lpFlsData); + std::lock_guard lk(g_flsMutex); if (dwFlsIndex < kMaxFlsValues && g_flsValuesUsed[dwFlsIndex]) { - g_flsValues[dwFlsIndex] = lpFlsData; + resetThreadFlsSlot(dwFlsIndex); + t_flsValues[dwFlsIndex].value = lpFlsData; return TRUE; } else { setLastError(ERROR_INVALID_PARAMETER); diff --git a/dll/kernel32/synchapi.cpp b/dll/kernel32/synchapi.cpp index c349faa..1bd144a 100644 --- a/dll/kernel32/synchapi.cpp +++ b/dll/kernel32/synchapi.cpp @@ -455,18 +455,24 @@ inline void setOwningThread(LPCRITICAL_SECTION crit, DWORD threadId) { } void waitForCriticalSection(LPCRITICAL_SECTION cs) { - auto *sequence = reinterpret_cast(&cs->LockSemaphore); - LONG observed = __atomic_load_n(sequence, __ATOMIC_ACQUIRE); - while (owningThreadId(cs) != 0) { - kernel32::WaitOnAddress(sequence, &observed, sizeof(observed), INFINITE); - observed = __atomic_load_n(sequence, __ATOMIC_ACQUIRE); + auto *tickets = reinterpret_cast(&cs->LockSemaphore); + for (;;) { + LONG available = __atomic_load_n(tickets, __ATOMIC_ACQUIRE); + if (available > 0) { + if (__atomic_compare_exchange_n(tickets, &available, available - 1, false, __ATOMIC_ACQ_REL, + __ATOMIC_ACQUIRE)) { + return; + } + continue; + } + kernel32::WaitOnAddress(tickets, &available, sizeof(available), INFINITE); } } void signalCriticalSection(LPCRITICAL_SECTION cs) { - auto *sequence = reinterpret_cast(&cs->LockSemaphore); - kernel32::InterlockedIncrement(const_cast(sequence)); - kernel32::WakeByAddressSingle(sequence); + auto *tickets = reinterpret_cast(&cs->LockSemaphore); + kernel32::InterlockedIncrement(const_cast(tickets)); + kernel32::WakeByAddressSingle(tickets); } inline bool trySpinAcquireCriticalSection(LPCRITICAL_SECTION cs, DWORD threadId) { @@ -1100,16 +1106,8 @@ void WINAPI LeaveCriticalSection(LPCRITICAL_SECTION lpCriticalSection) { return; } - const DWORD threadId = GetCurrentThreadId(); - if (owningThreadId(lpCriticalSection) != threadId || lpCriticalSection->RecursionCount <= 0) { - DEBUG_LOG("LeaveCriticalSection: thread %u does not own %p (owner=%u, recursion=%ld)\n", threadId, - lpCriticalSection, owningThreadId(lpCriticalSection), - static_cast(lpCriticalSection->RecursionCount)); - return; - } - auto *lockCount = const_cast(&lpCriticalSection->LockCount); - if (--lpCriticalSection->RecursionCount > 0) { + if (--lpCriticalSection->RecursionCount != 0) { kernel32::InterlockedDecrement(lockCount); return; } diff --git a/test/test_fls.c b/test/test_fls.c new file mode 100644 index 0000000..1368138 --- /dev/null +++ b/test/test_fls.c @@ -0,0 +1,89 @@ +#define _WIN32_WINNT 0x0600 + +#include "test_assert.h" +#include +#include + +#ifndef FLS_OUT_OF_INDEXES +#define FLS_OUT_OF_INDEXES ((DWORD)0xffffffff) +#endif + +typedef struct { + DWORD index; + HANDLE readyEvent; + HANDLE continueEvent; + int threadValue; + int reuseValue; +} FlsThreadContext; + +static DWORD WINAPI fls_thread_proc(LPVOID param) { + FlsThreadContext *ctx = (FlsThreadContext *)param; + TEST_CHECK(ctx != NULL); + + SetLastError(ERROR_GEN_FAILURE); + TEST_CHECK(FlsGetValue(ctx->index) == NULL); + TEST_CHECK_EQ(ERROR_SUCCESS, GetLastError()); + + TEST_CHECK(FlsSetValue(ctx->index, &ctx->threadValue)); + TEST_CHECK(FlsGetValue(ctx->index) == &ctx->threadValue); + TEST_CHECK(SetEvent(ctx->readyEvent)); + + TEST_CHECK_EQ(WAIT_OBJECT_0, WaitForSingleObject(ctx->continueEvent, 1000)); + + SetLastError(ERROR_GEN_FAILURE); + TEST_CHECK(FlsGetValue(ctx->index) == NULL); + TEST_CHECK_EQ(ERROR_SUCCESS, GetLastError()); + + TEST_CHECK(FlsSetValue(ctx->index, &ctx->reuseValue)); + TEST_CHECK(FlsGetValue(ctx->index) == &ctx->reuseValue); + + return 0; +} + +int main(void) { + DWORD index = FlsAlloc(NULL); + TEST_CHECK(index != FLS_OUT_OF_INDEXES); + + SetLastError(ERROR_GEN_FAILURE); + TEST_CHECK(FlsGetValue(index) == NULL); + TEST_CHECK_EQ(ERROR_SUCCESS, GetLastError()); + + int mainValue = 0x1234; + TEST_CHECK(FlsSetValue(index, &mainValue)); + TEST_CHECK(FlsGetValue(index) == &mainValue); + + FlsThreadContext ctx = {0}; + ctx.index = index; + ctx.threadValue = 0x5678; + ctx.reuseValue = 0x9abc; + ctx.readyEvent = CreateEventA(NULL, FALSE, FALSE, NULL); + ctx.continueEvent = CreateEventA(NULL, FALSE, FALSE, NULL); + TEST_CHECK(ctx.readyEvent != NULL); + TEST_CHECK(ctx.continueEvent != NULL); + + HANDLE thread = CreateThread(NULL, 0, fls_thread_proc, &ctx, 0, NULL); + TEST_CHECK(thread != NULL); + + TEST_CHECK_EQ(WAIT_OBJECT_0, WaitForSingleObject(ctx.readyEvent, 1000)); + TEST_CHECK(FlsGetValue(index) == &mainValue); + + TEST_CHECK(FlsFree(index)); + TEST_CHECK(FlsGetValue(index) == NULL); + + DWORD reusedIndex = FlsAlloc(NULL); + TEST_CHECK_EQ(index, reusedIndex); + + SetLastError(ERROR_GEN_FAILURE); + TEST_CHECK(FlsGetValue(reusedIndex) == NULL); + TEST_CHECK_EQ(ERROR_SUCCESS, GetLastError()); + + TEST_CHECK(SetEvent(ctx.continueEvent)); + TEST_CHECK_EQ(WAIT_OBJECT_0, WaitForSingleObject(thread, 1000)); + + TEST_CHECK(CloseHandle(thread)); + TEST_CHECK(CloseHandle(ctx.readyEvent)); + TEST_CHECK(CloseHandle(ctx.continueEvent)); + TEST_CHECK(FlsFree(reusedIndex)); + + return EXIT_SUCCESS; +}