Dmitry Timoshkov wrote:
>Hello,
>
>there is at least one application which crashes in CreateThread due
>to a race between a creating thread and a newly created one. The problem
>is that the tid address passed to CreateThread resides in the memory
>block allocated with VirtualAlloc which gets freed once the newly created
>thread starts running, and when CreateThread code attempts to store thread
>id it crashes.
>
>It appears that Windows always creates suspended threads in CreateThread
>and a created thread gets resumed later. Here is a snippet of the log
>running a simple test program under NT strace (the test app just creates
>a thread):
>
>hThread = CreateThread(NULL, 0, thread_proc, NULL, 0, &tid);
>CloseHandle(hThread);
>
>...
>117 796 792 NtCreateThread (0x1f03ff, 0x0, -1, 1243704, 1244568, 1, ... 60, {796, 800}, ) == 0x0
>118 796 792 NtRequestWaitReplyPort (40, {28, 52, new_msg, 0, 2013529912, 2147348480, 902, 2086471576} "\0\0\0\0\1\0\1\0x\1\23\0\320<\23\0<\0\0\0\34\3\0\0 \3\0\0" ... {28, 52, reply, 0, 796, 792, 2138, 0} "\0\0\0\0\1\0\1\0\0\0\0\0\320<\23\0<\0\0\0\34\3\0\0 \3\0\0" ) == 0x0
>119 796 792 NtResumeThread (60, ... 1, ) == 0x0
>120 796 792 NtClose (60, ... ) == 0x0
>...
>
>Changelog:
> Dmitry Timoshkov <dmitry(a)codeweavers.com>
> Always create a suspended thread in CreateThread and resume it
> if CREATE_SUSPENDED flag is not set as Windows does.
>
>--- cvs/hq/wine/dlls/kernel/thread.c 2004-09-22 16:48:59.000000000 +0900
>+++ wine/dlls/kernel/thread.c 2004-12-01 18:04:52.000000000 +0800
>@@ -164,7 +164,7 @@ HANDLE WINAPI CreateRemoteThread( HANDLE
> if (flags & STACK_SIZE_PARAM_IS_A_RESERVATION) stack_reserve = stack;
> else stack_commit = stack;
>
>- status = RtlCreateUserThread( hProcess, NULL, (flags & CREATE_SUSPENDED) != 0,
>+ status = RtlCreateUserThread( hProcess, NULL, TRUE,
> NULL, stack_reserve, stack_commit,
> THREAD_Start, info, &handle, &client_id );
> if (status == STATUS_SUCCESS)
>@@ -172,6 +172,19 @@ HANDLE WINAPI CreateRemoteThread( HANDLE
> if (id) *id = (DWORD)client_id.UniqueThread;
> if (sa && (sa->nLength >= sizeof(*sa)) && sa->bInheritHandle)
> SetHandleInformation( handle, HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT );
>+ if (!(flags & CREATE_SUSPENDED))
>+ {
>+ DWORD ret;
>+
>+ status = NtResumeThread( handle, &ret );
>+ if (status)
>+ {
>+ NtTerminateThread( handle, -1 );
>+ RtlFreeHeap( GetProcessHeap(), 0, info );
>+ SetLastError( RtlNtStatusToDosError(status) );
>+ handle = 0;
>+ }
>+ }
> }
> else
>
>
I think you should close the thread handle in the error case, otherwise
it will be leaked.
Rob