Always create a suspended thread in CreateThread and resume it if CREATE_SUSPENDED flag is not set as Windows does

Robert Shearman rob at codeweavers.com
Wed Dec 1 05:48:50 CST 2004


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 at 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



More information about the wine-devel mailing list