Prevent GetThreadContext failure (Windows)

(a cherry-pick of commits 3f39ea8, ab3699da from 'unity-master')

Calls to GetThreadContext may fail.  Work around this by putting
access in suspend/resume loop to advance thread past problematic areas
where suspend fails.  Capture context in per thread structure at
suspend time rather than retreiving it during the push logic.

* include/private/gcconfig.h [(I386 || X86_64) && (CYGWIN32 || MSWIN32)]
(RETRY_GET_THREAD_CONTEXT): Define macro.
* win32_threads.c [RETRY_GET_THREAD_CONTEXT]
(MAX_SUSPEND_THREAD_RETRIES): Likewise.
* include/private/gcconfig.h [NO_RETRY_GET_THREAD_CONTEXT]
(RETRY_GET_THREAD_CONTEXT): Undefine macro (for test purposes).
* win32_threads.c [RETRY_GET_THREAD_CONTEXT] (GC_Thread_Rep): Add
saved_context field; add comment.
* win32_threads.c [RETRY_GET_THREAD_CONTEXT] (GC_suspend): Define
retry_cnt local variable; set ContextFlags and call GetThreadContext()
after invocation of SuspendThread(); call ResumeThread() and proceed to
SuspendThread() if GetThreadContext() failed (the limit of iterations
is MAX_SUSPEND_THREAD_RETRIES); call Sleep(0) after ResumeThread() or
failed SuspendThread() except for the first 2 iterations.
* win32_threads.c [RETRY_GET_THREAD_CONTEXT] (GC_push_stack_for):
Declare pcontext local variable (which refers to thread->saved_context);
define context as a macro (to *pcontext) instead of a local variable;
add comment; do not set ContextFlags and do not call GetThreadContext();
undefine context after last use of PUSHn().

2 files changed, 70 insertions, 56 deletions

diff --git a/include/private/gcconfig.h b/include/private/gcconfig.h
index c1ef777d..90e30bc0 100644
--- a/include/private/gcconfig.h
+++ b/include/private/gcconfig.h
@@ -1531,6 +1531,7 @@ EXTERN_C_BEGIN
 #   ifdef CYGWIN32
 #       define OS_TYPE "CYGWIN32"
 #       define WOW64_THREAD_CONTEXT_WORKAROUND
+#       define RETRY_GET_THREAD_CONTEXT
 #       define DATASTART ((ptr_t)GC_DATASTART)  /* From gc.h */
 #       define DATAEND   ((ptr_t)GC_DATAEND)
 #       undef STACK_GRAN
@@ -1550,6 +1551,7 @@ EXTERN_C_BEGIN
 #   ifdef MSWIN32
 #       define OS_TYPE "MSWIN32"
 #       define WOW64_THREAD_CONTEXT_WORKAROUND
+#       define RETRY_GET_THREAD_CONTEXT
                 /* STACKBOTTOM and DATASTART are handled specially in   */
                 /* os_dep.c.                                            */
 #       define MPROTECT_VDB
@@ -2750,6 +2752,24 @@ EXTERN_C_BEGIN
 #         define HEAP_START DATAEND
 #       endif
 #   endif
+#   ifdef CYGWIN32
+#       define OS_TYPE "CYGWIN32"
+#       define RETRY_GET_THREAD_CONTEXT
+#       ifdef USE_WINALLOC
+#         define GWW_VDB
+#       else
+#         if defined(THREAD_LOCAL_ALLOC)
+            /* TODO: For an unknown reason, thread-local allocations    */
+            /* lead to spurious process exit after the fault handler is */
+            /* once invoked.                                            */
+#         else
+#           define MPROTECT_VDB
+#         endif
+#         ifdef USE_MMAP
+#           define USE_MMAP_ANON
+#         endif
+#       endif
+#   endif
 #   ifdef MSWIN_XBOX1
 #     define NO_GETENV
 #     define DATASTART (ptr_t)ALIGNMENT
@@ -2771,6 +2791,7 @@ EXTERN_C_BEGIN
 #   endif
 #   ifdef MSWIN32
 #       define OS_TYPE "MSWIN32"
+#       define RETRY_GET_THREAD_CONTEXT
                 /* STACKBOTTOM and DATASTART are handled specially in   */
                 /* os_dep.c.                                            */
 #       if !defined(__GNUC__) || defined(__INTEL_COMPILER) \
@@ -2865,6 +2886,10 @@ EXTERN_C_BEGIN
 # define USE_LIBC_PRIVATES
 #endif
 
+#ifdef NO_RETRY_GET_THREAD_CONTEXT
+# undef RETRY_GET_THREAD_CONTEXT
+#endif
+
 #if defined(LINUX_STACKBOTTOM) && defined(NO_PROC_STAT) \
     && !defined(USE_LIBC_PRIVATES)
     /* This combination will fail, since we have no way to get  */
diff --git a/win32_threads.c b/win32_threads.c
index 75aef0ea..0ed286aa 100644
--- a/win32_threads.c
+++ b/win32_threads.c
@@ -234,9 +234,6 @@ struct GC_Thread_Rep {
                                 /* memory (initially both are 0).       */
 
   unsigned char suspended; /* really of GC_bool type */
-  CONTEXT saved_context; /* populated as part of GC_suspend as          */
-                         /* resume/suspend loop may be needed for call  */
-                         /* to GetThreadContext to succeed              */
 
 # ifdef GC_PTHREADS
     unsigned char flags;        /* Protected by GC lock.                */
@@ -249,6 +246,12 @@ struct GC_Thread_Rep {
 #   define KNOWN_FINISHED(t) 0
 # endif
 
+# ifdef RETRY_GET_THREAD_CONTEXT
+    CONTEXT saved_context; /* Populated as part of GC_suspend() as      */
+                           /* resume/suspend loop may be needed for the */
+                           /* call to GetThreadContext() to succeed.    */
+# endif
+
 # ifdef THREAD_LOCAL_ALLOC
     struct thread_local_freelists tlfs;
 # endif
@@ -1180,8 +1183,9 @@ void GC_push_thread_structures(void)
 /* Suspend the given thread, if it's still active.      */
 STATIC void GC_suspend(GC_thread t)
 {
-# ifndef MSWINCE
-  int iterations;
+# ifdef RETRY_GET_THREAD_CONTEXT
+    int retry_cnt = 0;
+#   define MAX_SUSPEND_THREAD_RETRIES (1000 * 1000)
 # endif
   UNPROTECT_THREAD(t);
   GC_acquire_dirty_lock ();
@@ -1189,55 +1193,30 @@ STATIC void GC_suspend(GC_thread t)
     /* SuspendThread() will fail if thread is running kernel code.      */
     while (SuspendThread(THREAD_HANDLE(t)) == (DWORD)-1)
       Sleep(10); /* in millis */
-# else
-    iterations = 0;
-    while (TRUE)
-    {
-      /* Apparently the Windows 95 GetOpenFileName call creates           */
-      /* a thread that does not properly get cleaned up, and              */
-      /* SuspendThread on its descriptor may provoke a crash.             */
-      /* This reduces the probability of that event, though it still      */
-      /* appears there's a race here.                                     */
-      DWORD exitCode;
-
-      if (GetExitCodeThread (t->handle, &exitCode) &&
-          exitCode != STILL_ACTIVE)
-      {
-#       ifdef GC_PTHREADS
-          t->stack_base = 0; /* prevent stack from being pushed */
-#       else
-          /* this breaks pthread_join on Cygwin, which is guaranteed to  */
-          /* only see user pthreads                                      */
-          GC_ASSERT (GC_win32_dll_threads);
-          GC_delete_gc_thread_no_free (t);
-#       endif
-        GC_release_dirty_lock ();
-        return;
-      }
-
-      {
-        DWORD res;
-        do {
-          res = SuspendThread (t->handle);
-        } while (res == (DWORD)-1);
-      }
+# elif defined(RETRY_GET_THREAD_CONTEXT)
+    for (;;) {
+      if (SuspendThread(t->handle) != (DWORD)-1) {
+        t->saved_context.ContextFlags = GET_THREAD_CONTEXT_FLAGS;
+        if (GetThreadContext(t->handle, &t->saved_context)) {
+          /* TODO: WoW64 extra workaround: if CONTEXT_EXCEPTION_ACTIVE  */
+          /* then Sleep(1) and retry.                                   */
+          break; /* success; the context is saved */
+        }
 
-      t->saved_context.ContextFlags = CONTEXT_INTEGER | CONTEXT_CONTROL;
-      if (GetThreadContext (t->handle, &t->saved_context)) {
-        t->suspended = (unsigned char)TRUE;
-        break; /* success case break out of loop */
+        /* Resume the thread, try to suspend it in a better location.   */
+        if (ResumeThread(t->handle) == (DWORD)-1)
+          ABORT("ResumeThread failed");
       }
-
-      /* resume thread and try to suspend in better location */
-      if (ResumeThread (t->handle) == (DWORD)-1)
-        ABORT ("ResumeThread failed");
-
-      /* after a million tries something must be wrong */
-      if (iterations++ == 1000 * 1000)
-        ABORT ("SuspendThread loop failed");
+      if (retry_cnt > 1)
+        Sleep(0); /* yield */
+      if (++retry_cnt >= MAX_SUSPEND_THREAD_RETRIES)
+        ABORT("SuspendThread loop failed"); /* something must be wrong */
     }
-# endif /* !MSWINCE */
-  t->suspended = (unsigned char)TRUE;
+# else
+    if (SuspendThread(t -> handle) == (DWORD)-1)
+      ABORT("SuspendThread failed");
+# endif
+  t -> suspended = (unsigned char)TRUE;
   GC_release_dirty_lock();
   if (GC_on_thread_event)
     GC_on_thread_event(GC_EVENT_THREAD_SUSPENDED, THREAD_HANDLE(t));
@@ -1436,12 +1415,19 @@ STATIC word GC_push_stack_for(GC_thread thread, DWORD me)
     sp = GC_approx_sp();
   } else if ((sp = thread -> thread_blocked_sp) == NULL) {
               /* Use saved sp value for blocked threads. */
-    /* For unblocked threads call GetThreadContext().   */
-    CONTEXT context;
+#   ifdef RETRY_GET_THREAD_CONTEXT
+      /* We cache context when suspending the thread since it may       */
+      /* require looping.                                               */
+      CONTEXT *pcontext = &thread->saved_context;
+#     define context (*pcontext)
+#   else
+      /* For unblocked threads call GetThreadContext(). */
+      CONTEXT context;
 
-    context.ContextFlags = GET_THREAD_CONTEXT_FLAGS;
-    if (!GetThreadContext(THREAD_HANDLE(thread), &context))
-      ABORT("GetThreadContext failed");
+      context.ContextFlags = GET_THREAD_CONTEXT_FLAGS;
+      if (!GetThreadContext(THREAD_HANDLE(thread), &context))
+        ABORT("GetThreadContext failed");
+#   endif
 
     /* Push all registers that might point into the heap.  Frame        */
     /* pointer registers are included in case client code was           */
@@ -1544,6 +1530,9 @@ STATIC word GC_push_stack_for(GC_thread thread, DWORD me)
         }
 #     endif
 #   endif /* WOW64_THREAD_CONTEXT_WORKAROUND */
+#   ifdef RETRY_GET_THREAD_CONTEXT
+#     undef context
+#   endif
   } /* ! current thread */
 
   /* Set stack_min to the lowest address in the thread stack,   */