Problem (and fix) with thread creation in Windows.

[jridges@xxxxxxxxxx (John Ridges)]

Hi Benny,

I suggest you make the following changes to "os_core_win32.c" in pjlib:


--- os_core_win32.c    Wed Sep 22 14:45:00 2010
+++ new\os_core_win32.c    Fri Oct 29 10:53:41 2010
@@ -29,6 +29,7 @@
  #include <stddef.h>
  #include <stdlib.h>
  #include <stdio.h>
+#include <process.h>

  #if defined(PJ_HAS_WINSOCK_H) && PJ_HAS_WINSOCK_H != 0
  #  include <winsock.h>
@@ -450,16 +451,11 @@
                                        unsigned flags,
                                        pj_thread_t **thread_ptr)
  {
-    DWORD dwflags = 0;
      pj_thread_t *rec;

      PJ_CHECK_STACK();
      PJ_ASSERT_RETURN(pool && proc && thread_ptr, PJ_EINVAL);

-    /* Set flags */
-    if (flags & PJ_THREAD_SUSPENDED)
-        dwflags |= CREATE_SUSPENDED;
-
      /* Create thread record and assign name for the thread */
      rec = (struct pj_thread_t*) pj_pool_calloc(pool, 1, 
sizeof(pj_thread_t));
      if (!rec)
@@ -486,11 +482,15 @@
      /* Create the thread. */
      rec->proc = proc;
      rec->arg = arg;
-    rec->hthread = CreateThread(NULL, stack_size,
-                                thread_main, rec,
-                                dwflags, &rec->idthread);
+    rec->hthread = (HANDLE)_beginthreadex(NULL, stack_size,
+                                (unsigned (__stdcall *)(void 
*))thread_main, rec,
+                                CREATE_SUSPENDED, 
&(unsigned)rec->idthread);
      if (rec->hthread == NULL)
          return PJ_RETURN_OS_ERROR(GetLastError());
+
+    /* If flags don't request suspended, resume the thread */
+    if (!(flags & PJ_THREAD_SUSPENDED))
+        ResumeThread(rec->hthread);

      /* Success! */
      *thread_ptr = rec;


The first change uses the C library function "_beginthreadex" to create 
a thread instead of the Windows function "CreateThread". Since (in 
Windows at least) pjlib links with functions in the C run-time library, 
Microsoft requires that you use "_beginthreadex" so that certain C 
run-time library variables can be initialized. Plus it would have the 
added benefit that the user would get a link error if he inadvertently 
linked with the single-threaded versions of the C run-time library.

The second change eliminates a race condition (that has actually 
happened to me). When a thread is created, it sometimes immediately 
executes before the system call that created the thread returns, so if 
the thread function makes a call like 
"pj_thread_get_prio(pj_thread_this())" it will get the wrong value 
because "rec->hthread" hasn't been assigned yet in "pj_thread_create". 
The solution is to create the thread suspended so that it won't run 
until after the thread handle has been assigned.

Cheers,

John Ridges