Fix for rare mono runtime crash during shutdown sequence on Windows.
authorlateralusX <lateralusx.github@gmail.com>
Wed, 18 May 2016 10:20:55 +0000 (12:20 +0200)
committerlateralusX <lateralusx.github@gmail.com>
Wed, 18 May 2016 10:20:55 +0000 (12:20 +0200)
commitbe0e8038120c0bdcd161597daf43c8c2ba7696ce
tree144048069cdf8dd95686ca6fe4d537d687529995
parentd70777a3332af2d630d24adf620c2e548b92b56a
Fix for rare mono runtime crash during shutdown sequence on Windows.

During mono shutdown on windows there is, on rare occasions, an exception
in CloseHandle indicating the closure of an invalid handle. This exception
is normally a strong indication of a much more severe underlying problem,
a doubled close on a handle, that could lead to random data corruptions
and very hard to track bugs in random areas of the code. If the system reused
the handle for a different object that silently gets incorrectly closed by above
call, the above bug could manifest itself into some completely different issues,
so getting a consistent repro with the above exception with CloseHandle was a good
point to start tracking down the bug.

The exception throwed in CloseHandle boils down to the fact that threads
in thread pool's that have their finalizer run on shutdown can end up in
a scenario where the thread handle has already been closed elsewhere.

When the threads are finalized in mono_gc_finialize_threadpool_threads,
the handle on the internal thread object, stored in MonoInternalThread,
will be closed by the thread objects finializer in ves_icall_System_Threading_InternalThread_Thread_free_internal.
Under some rare occasions, one of the handles are not valid and will cause
the exception in CloseHandle or even worse, pointing to a complete different
kind of object, potentially closed by the threads finalizer, causing an instable
process with random behavior related to WIN32 objects (files, mutexes, threads, events etc).

The reason why it rarely manifests itself is due to a race between the finializer thread and the
main thread during shutdown. Main thread gives finalizer thread 2000 ms to complete its work and
if it’s time outs, the main thread won’t call the code that will run the finalizers,
mono_gc_finalize_threadpool_threads for the thread objects added to an internal list
by the finalizer thread, so if the finalizer takes more than 2000 ms to complete during shutdown
the bug won’t manifest itself. By increasing the wait time in mono_domain_finalize to for example INFINITE,
the repro of this bug will be 100% and not timing dependent anymore.

Turns out that the offending CloseHandle on the thread object happens at an
earlier point in time during shutdown, when mono_thread_manage tries to wait for
threads to complete manage thread it will do remove_and_abort_threads for background
threads still around. The way this works is that it will setup a list of thread handles
that it will wait for using WaitForMultipleObjects and then close the handles.
It turns out that the implementation of remove_and_abort_threads adds the wrong
handle to the list, not the duplicated handle but the original thread handle,
and that will trigger this bug, since the original thread handle will be closed
(and the duplicated handle will leak). The fix is to use the duplicated thread
handle instead of the original, similar to how it was already done in other similar methods, like build_wait_tids.
Sinces this bug seems to been around for quite some time and is a race related to managed threads and mono
shutdown sequence it could give other weird side effects on windows during shutdown
(since it could have closed a random WIN32 object instead of the intended thread).

Looking at Bugzilla, there is one bug that very well could be related to this specific problem, 10108:

https://bugzilla.xamarin.com/show_bug.cgi?id=10108

This bug should most likely be retested with this fix to see if it still reproducible once merged into master.
mono/metadata/threads.c