Author: ion Date: Sat Jul 23 11:28:35 2011 New Revision: 52800
URL: http://svn.reactos.org/svn/reactos?rev=52800&view=rev Log: [KERNEL32]: Bug #35: TlsSetValue should allocate the TlsExpansionSlots under the PEB Lock. Bug #36: TlsGetValue needs to set the last error to ERROR_SUCCESS when the index is valid (amd even if no expansion slots exist!). Bug #37: TlsFree did not check the return of NtSetInformationThread(ThreadZeroTlsCell). Bug #38: TlsAlloc was setting error to ERROR_NO_MORE_ITEMS instead of STATUS_NO_MEMORY. In principle the former is more accurate, but that's not how API compatibility works. Optimize TLS functions not to call NtQueryTeb/Peb all the time. Cache the TEB/PEB value instead.
Modified: trunk/reactos/dll/win32/kernel32/client/synch.c trunk/reactos/dll/win32/kernel32/client/thread.c
Modified: trunk/reactos/dll/win32/kernel32/client/synch.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/kernel32/client/s... ============================================================================== --- trunk/reactos/dll/win32/kernel32/client/synch.c [iso-8859-1] (original) +++ trunk/reactos/dll/win32/kernel32/client/synch.c [iso-8859-1] Sat Jul 23 11:28:35 2011 @@ -39,7 +39,7 @@ LARGE_INTEGER Time; NTSTATUS Status; RTL_CALLER_ALLOCATED_ACTIVATION_CONTEXT_STACK_FRAME ActCtx; - + /* APCs must execute with the default activation context */ if (bAlertable) { @@ -75,7 +75,7 @@ Status = WAIT_FAILED; } } while ((Status == STATUS_ALERTED) && (bAlertable)); - + /* Cleanup the activation context */ if (bAlertable) RtlDeactivateActivationContextUnsafeFast(&ActCtx);
@@ -119,7 +119,7 @@ DWORD i; NTSTATUS Status; RTL_CALLER_ALLOCATED_ACTIVATION_CONTEXT_STACK_FRAME ActCtx; - + /* APCs must execute with the default activation context */ if (bAlertable) { @@ -196,7 +196,7 @@
/* Cleanup the activation context */ if (bAlertable) RtlDeactivateActivationContextUnsafeFast(&ActCtx); - + /* Return wait status */ return Status; } @@ -215,7 +215,7 @@ LARGE_INTEGER Time; NTSTATUS Status; RTL_CALLER_ALLOCATED_ACTIVATION_CONTEXT_STACK_FRAME ActCtx; - + /* APCs must execute with the default activation context */ if (bAlertable) { @@ -225,7 +225,7 @@ ActCtx.Format = RTL_CALLER_ALLOCATED_ACTIVATION_CONTEXT_STACK_FRAME_FORMAT_WHISTLER; RtlActivateActivationContextUnsafeFast(&ActCtx, NULL); } - + /* Get real handle */ hObjectToWaitOn = TranslateStdHandle(hObjectToWaitOn);
@@ -258,7 +258,7 @@
/* Cleanup the activation context */ if (bAlertable) RtlDeactivateActivationContextUnsafeFast(&ActCtx); - + /* Return wait status */ return Status; } @@ -674,7 +674,7 @@ PLARGE_INTEGER TimePtr; NTSTATUS errCode; RTL_CALLER_ALLOCATED_ACTIVATION_CONTEXT_STACK_FRAME ActCtx; - + /* APCs must execute with the default activation context */ if (bAlertable) { @@ -702,7 +702,7 @@ errCode = NtDelayExecution((BOOLEAN)bAlertable, TimePtr); } while ((bAlertable) && (errCode == STATUS_ALERTED)); - + /* Return the correct code */ return (errCode == STATUS_USER_APC) ? WAIT_IO_COMPLETION : 0; }
Modified: trunk/reactos/dll/win32/kernel32/client/thread.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/kernel32/client/t... ============================================================================== --- trunk/reactos/dll/win32/kernel32/client/thread.c [iso-8859-1] (original) +++ trunk/reactos/dll/win32/kernel32/client/thread.c [iso-8859-1] Sat Jul 23 11:28:35 2011 @@ -230,7 +230,7 @@ { ASSERT(FALSE); } - + /* Success */ if(lpThreadId) *lpThreadId = HandleToUlong(ClientId.UniqueThread);
@@ -939,51 +939,61 @@ TlsAlloc(VOID) { ULONG Index; - + PTEB Teb; + PPEB Peb; + + /* Get the PEB and TEB, lock the PEB */ + Teb = NtCurrentTeb(); + Peb = Teb->ProcessEnvironmentBlock; RtlAcquirePebLock();
- /* Try to get regular TEB slot. */ - Index = RtlFindClearBitsAndSet(NtCurrentPeb()->TlsBitmap, 1, 0); - if (Index == ~0U) - { - /* If it fails, try to find expansion TEB slot. */ - Index = RtlFindClearBitsAndSet(NtCurrentPeb()->TlsExpansionBitmap, 1, 0); - if (Index != ~0U) + /* Try to get regular TEB slot */ + Index = RtlFindClearBitsAndSet(Peb->TlsBitmap, 1, 0); + if (Index != 0xFFFFFFFF) + { + /* Clear the value. */ + Teb->TlsSlots[Index] = 0; + RtlReleasePebLock(); + return Index; + } + + /* If it fails, try to find expansion TEB slot. */ + Index = RtlFindClearBitsAndSet(Peb->TlsExpansionBitmap, 1, 0); + if (Index != 0xFFFFFFFF) + { + /* Is there no expansion slot yet? */ + if (!Teb->TlsExpansionSlots) { - if (NtCurrentTeb()->TlsExpansionSlots == NULL) - { - NtCurrentTeb()->TlsExpansionSlots = HeapAlloc(GetProcessHeap(), - HEAP_ZERO_MEMORY, - TLS_EXPANSION_SLOTS * - sizeof(PVOID)); - } - - if (NtCurrentTeb()->TlsExpansionSlots == NULL) - { - RtlClearBits(NtCurrentPeb()->TlsExpansionBitmap, Index, 1); - Index = ~0; - SetLastError(ERROR_NOT_ENOUGH_MEMORY); - } - else - { - /* Clear the value. */ - NtCurrentTeb()->TlsExpansionSlots[Index] = 0; - Index += TLS_MINIMUM_AVAILABLE; - } + /* Allocate an array */ + Teb->TlsExpansionSlots = RtlAllocateHeap(RtlGetProcessHeap(), + HEAP_ZERO_MEMORY, + TLS_EXPANSION_SLOTS * + sizeof(PVOID)); + } + + /* Did we get an array? */ + if (!Teb->TlsExpansionSlots) + { + /* Fail */ + RtlClearBits(Peb->TlsExpansionBitmap, Index, 1); + Index = 0xFFFFFFFF; + SetLastErrorByStatus(STATUS_NO_MEMORY); } else { - SetLastError(ERROR_NO_MORE_ITEMS); + /* Clear the value. */ + Teb->TlsExpansionSlots[Index] = 0; + Index += TLS_MINIMUM_AVAILABLE; } } else { - /* Clear the value. */ - NtCurrentTeb()->TlsSlots[Index] = 0; - } - + /* Fail */ + SetLastErrorByStatus(STATUS_NO_MEMORY); + } + + /* Release the lock and return */ RtlReleasePebLock(); - return Index; }
@@ -992,52 +1002,70 @@ */ BOOL WINAPI -TlsFree(DWORD Index) +TlsFree(IN DWORD Index) { BOOL BitSet; - - if (Index >= TLS_EXPANSION_SLOTS + TLS_MINIMUM_AVAILABLE) - { + PPEB Peb; + ULONG TlsIndex; + PVOID TlsBitmap; + NTSTATUS Status; + + /* Acquire the PEB lock and grab the PEB */ + Peb = NtCurrentPeb(); + RtlAcquirePebLock(); + + /* Check if the index is too high */ + if (Index >= TLS_MINIMUM_AVAILABLE) + { + /* Check if it can fit in the expansion slots */ + TlsIndex = Index - TLS_MINIMUM_AVAILABLE; + if (TlsIndex >= TLS_EXPANSION_SLOTS) + { + /* It's invalid */ + SetLastErrorByStatus(STATUS_INVALID_PARAMETER); + RtlReleasePebLock(); + return FALSE; + } + else + { + /* Use the expansion bitmap */ + TlsBitmap = Peb->TlsExpansionBitmap; + Index = TlsIndex; + } + } + else + { + /* Use the normal bitmap */ + TlsBitmap = Peb->TlsBitmap; + } + + /* Check if the index was set */ + BitSet = RtlAreBitsSet(TlsBitmap, Index, 1); + if (BitSet) + { + /* Tell the kernel to free the TLS cells */ + Status = NtSetInformationThread(NtCurrentThread(), + ThreadZeroTlsCell, + &Index, + sizeof(DWORD)); + if (!NT_SUCCESS(Status)) + { + SetLastErrorByStatus(STATUS_INVALID_PARAMETER); + return FALSE; + } + + /* Clear the bit */ + RtlClearBits(TlsBitmap, Index, 1); + } + else + { + /* Fail */ SetLastErrorByStatus(STATUS_INVALID_PARAMETER); return FALSE; }
- RtlAcquirePebLock(); - - if (Index >= TLS_MINIMUM_AVAILABLE) - { - BitSet = RtlAreBitsSet(NtCurrentPeb()->TlsExpansionBitmap, - Index - TLS_MINIMUM_AVAILABLE, - 1); - - if (BitSet) - RtlClearBits(NtCurrentPeb()->TlsExpansionBitmap, - Index - TLS_MINIMUM_AVAILABLE, - 1); - } - else - { - BitSet = RtlAreBitsSet(NtCurrentPeb()->TlsBitmap, Index, 1); - if (BitSet) - RtlClearBits(NtCurrentPeb()->TlsBitmap, Index, 1); - } - - if (BitSet) - { - /* Clear the TLS cells (slots) in all threads of the current process. */ - NtSetInformationThread(NtCurrentThread(), - ThreadZeroTlsCell, - &Index, - sizeof(DWORD)); - } - else - { - SetLastError(ERROR_INVALID_PARAMETER); - } - - RtlReleasePebLock(); - - return BitSet; + /* Done! */ + return TRUE; }
/* @@ -1045,67 +1073,96 @@ */ LPVOID WINAPI -TlsGetValue(DWORD Index) -{ +TlsGetValue(IN DWORD Index) +{ + PTEB Teb; + + /* Get the TEB and clear the last error */ + Teb = NtCurrentTeb(); + Teb->LastErrorValue = 0; + + /* Check for simple TLS index */ + if (Index < TLS_MINIMUM_AVAILABLE) + { + /* Return it */ + return Teb->TlsSlots[Index]; + } + + /* Check for valid index */ if (Index >= TLS_EXPANSION_SLOTS + TLS_MINIMUM_AVAILABLE) { + /* Fail */ SetLastErrorByStatus(STATUS_INVALID_PARAMETER); return NULL; }
- SetLastError(NO_ERROR); - - if (Index >= TLS_MINIMUM_AVAILABLE) - { - /* The expansion slots are allocated on demand, so check for it. */ - if (NtCurrentTeb()->TlsExpansionSlots == NULL) - return NULL; - return NtCurrentTeb()->TlsExpansionSlots[Index - TLS_MINIMUM_AVAILABLE]; - } - else - { - return NtCurrentTeb()->TlsSlots[Index]; - } -} - -/* - * @implemented - */ -BOOL -WINAPI -TlsSetValue(DWORD Index, - LPVOID Value) -{ - if (Index >= TLS_EXPANSION_SLOTS + TLS_MINIMUM_AVAILABLE) - { + /* The expansion slots are allocated on demand, so check for it. */ + Teb->LastErrorValue = 0; + if (!Teb->TlsExpansionSlots) return NULL; + + /* Return the value from the expansion slots */ + return Teb->TlsExpansionSlots[Index - TLS_MINIMUM_AVAILABLE]; +} + +/* + * @implemented + */ +BOOL +WINAPI +TlsSetValue(IN DWORD Index, + IN LPVOID Value) +{ + DWORD TlsIndex; + PTEB Teb = NtCurrentTeb(); + + /* Check for simple TLS index */ + if (Index < TLS_MINIMUM_AVAILABLE) + { + /* Return it */ + Teb->TlsSlots[Index] = Value; + return TRUE; + } + + /* Check if this is an expansion slot */ + TlsIndex = Index - TLS_MINIMUM_AVAILABLE; + if (TlsIndex >= TLS_EXPANSION_SLOTS) + { + /* Fail */ SetLastErrorByStatus(STATUS_INVALID_PARAMETER); return FALSE; }
- if (Index >= TLS_MINIMUM_AVAILABLE) - { - if (NtCurrentTeb()->TlsExpansionSlots == NULL) + /* Do we not have expansion slots? */ + if (!Teb->TlsExpansionSlots) + { + /* Get the PEB lock to see if we still need them */ + RtlAcquirePebLock(); + if (!Teb->TlsExpansionSlots) { - NtCurrentTeb()->TlsExpansionSlots = HeapAlloc(GetProcessHeap(), - HEAP_ZERO_MEMORY, - TLS_EXPANSION_SLOTS * - sizeof(PVOID)); - - if (NtCurrentTeb()->TlsExpansionSlots == NULL) + /* Allocate them */ + Teb->TlsExpansionSlots = RtlAllocateHeap(RtlGetProcessHeap(), + HEAP_ZERO_MEMORY, + TLS_EXPANSION_SLOTS * + sizeof(PVOID)); + if (!Teb->TlsExpansionSlots) { - SetLastError(ERROR_NOT_ENOUGH_MEMORY); + /* Fail */ + RtlReleasePebLock(); + SetLastErrorByStatus(STATUS_NO_MEMORY); return FALSE; } }
- NtCurrentTeb()->TlsExpansionSlots[Index - TLS_MINIMUM_AVAILABLE] = Value; - } - else - { - NtCurrentTeb()->TlsSlots[Index] = Value; - } - - return TRUE; -} + /* Release the lock */ + RtlReleasePebLock(); + } + + /* Write the value */ + Teb->TlsExpansionSlots[TlsIndex] = Value; + + /* Success */ + return TRUE; +} +
/* EOF */