diff options
author | Sergiy Kuryata <sergeyk@microsoft.com> | 2017-03-24 21:37:17 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-03-24 21:37:17 +0300 |
commit | 52d269eb6dadbf96c6111fb998dcf7025bc83ba0 (patch) | |
tree | dd9b5af0c854900162a68a1cf6810248f999bb47 /src/Native/Runtime/unix | |
parent | dd7bc60e519ea0bb6951f25d8d171a326f51277c (diff) |
Fix a race in thread shutdown on Unix (#3073)
* Fix a race in thread shutdown on Unix
The problem is that the _stopped event (that is used for signaling that
the thread has stopped) is allocated in the CreateThread() method after an
OS thread is created. As a result, the parent thread can be scheduled out
before it allocates an event and assigns it _stopped. At same time, the
newly created thread gets a chance to run, finishes its work promptly and
exits; while exiting, it tries to signal the _stopped event but it crashes
instead because the event has not been allocated yet. I believe the
problem with lifetime management of the _stopped event is due to the fact
that the original code relied on the value of _stopped in the HasStarted()
method to indicate whether a thread has been created/started or not. On
Windows, the HasStarted() method relied on the value of _osHandle for the
same purpose.
This change fixes this problem by:
1. Changing the HasStarted() method to use the explicit state of the tread
ThreadState.Unstarted (i.e. the _threadState field). This also makes
implementation of this method platform independent. The existing code
already sets the thread state to ThreadState.Unstarted (when an instance
of the RuntimeThread class is created) and clears the bit when the thread
starts executing managed code.
2. Changing RuntimeThread on Unix to allocate the stopped event before
thread creation to actually fix the race.
I have also partially enabled the BasicThreading test for Unix and added a
small regression test for the race condition. Full functionality of
BasicThreading will be enabled once we have ThreadPool supported on all
platforms.
* PR Feedback
* Fix issue with __cxa_thread_atexit in PAL. The change (https://github.com/dotnet/corert/pull/2531)
that introduced __cxa_thread_atexit in PAL did it incorrectly. The __cxa_thread_atexit function
should be called for each TLS object that needs its destructor to be executed at thread shutdown time.
The incorrect usage of __cxa_thread_atexit resulted in hangs in SuspendEE and random corruptions.
Diffstat (limited to 'src/Native/Runtime/unix')
-rw-r--r-- | src/Native/Runtime/unix/PalRedhawkUnix.cpp | 6 |
1 files changed, 2 insertions, 4 deletions
diff --git a/src/Native/Runtime/unix/PalRedhawkUnix.cpp b/src/Native/Runtime/unix/PalRedhawkUnix.cpp index 95ba38f7d..2c59cdb22 100644 --- a/src/Native/Runtime/unix/PalRedhawkUnix.cpp +++ b/src/Native/Runtime/unix/PalRedhawkUnix.cpp @@ -443,10 +443,6 @@ REDHAWK_PALEXPORT bool REDHAWK_PALAPI PalInit() } #endif // !USE_PORTABLE_HELPERS -#if !HAVE_THREAD_LOCAL - __cxa_thread_atexit(RuntimeThreadShutdown, NULL, &__dso_handle); -#endif - return true; } @@ -494,6 +490,8 @@ extern "C" void PalAttachThread(void* thread) { #if HAVE_THREAD_LOCAL tls_destructionMonitor.SetThread(thread); +#else + __cxa_thread_atexit(RuntimeThreadShutdown, thread, &__dso_handle); #endif } |