Skip to content

Commit 15d6da0

Browse files
committed
Move thread exit check within loop.
Avoids race condition between checking for exit, thread exiting, and thread suspension logic.
1 parent 4ac9f57 commit 15d6da0

File tree

1 file changed

+32
-34
lines changed

1 file changed

+32
-34
lines changed

libgc/win32_threads.c

+32-34
Original file line numberDiff line numberDiff line change
@@ -304,44 +304,42 @@ void GC_stop_world()
304304
Sleep(10);
305305
thread_table[i].suspended = TRUE;
306306
# else
307-
/* Apparently the Windows 95 GetOpenFileName call creates */
308-
/* a thread that does not properly get cleaned up, and */
309-
/* SuspendThread on its descriptor may provoke a crash. */
310-
/* This reduces the probability of that event, though it still */
311-
/* appears there's a race here. */
312-
DWORD exitCode;
313-
if (GetExitCodeThread(thread_table[i].handle,&exitCode) &&
314-
exitCode != STILL_ACTIVE) {
307+
308+
while (InterlockedCompareExchange (&thread_table[i].in_use, 0, 0)) {
309+
/* Apparently the Windows 95 GetOpenFileName call creates */
310+
/* a thread that does not properly get cleaned up, and */
311+
/* SuspendThread on its descriptor may provoke a crash. */
312+
/* This reduces the probability of that event, though it still */
313+
/* appears there's a race here. */
314+
DWORD exitCode;
315+
if (GetExitCodeThread (thread_table[i].handle, &exitCode) && exitCode != STILL_ACTIVE) {
315316
thread_table[i].stack_base = 0; /* prevent stack from being pushed */
316317
# ifndef CYGWIN32
317-
/* this breaks pthread_join on Cygwin, which is guaranteed to */
318-
/* only see user pthreads */
319-
thread_table[i].in_use = FALSE;
320-
CloseHandle(thread_table[i].handle);
318+
/* this breaks pthread_join on Cygwin, which is guaranteed to */
319+
/* only see user pthreads */
320+
InterlockedExchange (&thread_table[i].in_use, FALSE);
321+
CloseHandle (thread_table[i].handle);
321322
# endif
322-
continue;
323-
}
323+
break; /* thread has exited mark it as unused and break out of while loop */
324+
}
324325

325-
while (InterlockedCompareExchange (&thread_table[i].in_use, 0, 0)) {
326-
DWORD res;
327-
do {
328-
res = SuspendThread (thread_table[i].handle);
329-
} while (res == -1);
330-
331-
thread_table[i].saved_context.ContextFlags = CONTEXT_INTEGER | CONTEXT_CONTROL;
332-
if (GetThreadContext (thread_table[i].handle, &thread_table[i].saved_context)) {
333-
thread_table[i].suspended = TRUE;
334-
break; /* success case break out of loop */
335-
}
336-
337-
/* resume thread and try to suspend in better location */
338-
if (ResumeThread (thread_table[i].handle) == (DWORD)-1)
339-
ABORT ("ResumeThread failed");
340-
341-
/* after a million tries something must be wrong */
342-
if (iterations++ == 1000 * 1000)
343-
ABORT ("SuspendThread loop failed");
344-
}
326+
if (SuspendThread (thread_table[i].handle) == -1)
327+
continue; /* retry suspension of thread if it's still running */
328+
329+
thread_table[i].saved_context.ContextFlags = CONTEXT_INTEGER | CONTEXT_CONTROL;
330+
if (GetThreadContext (thread_table[i].handle, &thread_table[i].saved_context)) {
331+
thread_table[i].suspended = TRUE;
332+
break; /* success case break out of loop */
333+
}
334+
335+
/* resume thread and try to suspend in better location */
336+
if (ResumeThread (thread_table[i].handle) == (DWORD)-1)
337+
ABORT ("ResumeThread failed");
338+
339+
/* after a million tries something must be wrong */
340+
if (iterations++ == 1000 * 1000)
341+
ABORT ("SuspendThread loop failed");
342+
}
345343

346344
# endif
347345
}

0 commit comments

Comments
 (0)