review.tizen.org Git - platform/upstream/dotnet/runtime.git/log

projects / platform / upstream / dotnet / runtime.git / log

monojenkins [Sat, 17 Oct 2020 12:04:40 +0000 (08:04 -0400)]

[threads] At shutdown, don't wait for native threads that aren't calling Mono (#43174)

* [test] Call P/Invoke callback delegates from foreign threads

Change post-detach-1.cs to also have versions that call reverse pinvokes from
foreign threads that were not attached to mono.

The foreign threads should not prevent GC and should not prevent Mono from
shutting down using mono_manage_internal.

* [threads] Don't wait for native threads that aren't calling Mono

If a thread is started from native code, and it interacts with the runtime (by
calling a thunk that invokes a managed method), the runtime will attach the
thread - it will create a `MonoInternalThread` and add it to the list of
threads hash (in threads.c).

If the thread returns from the managed call, it will still be recorded by the
runtime, but as long as it is not running managed code anymore, it will prevent
shutdown.  The problem is when we try to suspend the thread in order to abort
it, `mono_thread_state_init_from_handle` will see a NULL domain (because
`mono_threads_detach_coop_internal` will restore it to NULL when a managed
method returns back to native code).  (On systems using POSIX signals to
suspend, the same check is in `mono_thread_state_init_from_sigctx`).  As a
result, `mono_threads_suspend_begin_async_suspend` (or `suspend_signal_handler`
on POSIX) will set `suspend_can_continue` to FALSE, and
`mono_thread_info_safe_suspend_and_run` will not run the suspend callback.

As a result, when `mono_manage_internal` calls `abort_threads`, it will add the
thread handle to the wait list, but it will not actually request the thread to
abort.  As a result, after `abort_threads` returns, the subsequent call to
`wait_for_tids` will block until the native thread terminates (at which point
the TLS destructor will notify the thread handle and wait_for_tids will
unblock).

This commit changes the behavior of `abort_threads` to ignore threads that do
not run `async_suspend_critical` and not to add them to the wait list.  As a
result, if a native thread calls into managed and then returns to native code,
the runtime will not wait for it.

* [threads] Warn if mono_thread_manage_internal can't abort a thread

Give a hint to embedders to aid debugging

* rename AbortThreadData:thread_will_abort field

It's used to keep track of whether the thread will eventually throw a TAE (and
thus that we need to wait for it).

The issue is that under full coop suspend, we treat threads in GC
Safe (BLOCKING) state as if they're suspended and always execute
async_abort_critical.  So the field has nothing to do with whether the thread
was suscessfully suspended, but rather whether it will (eventually) be aborted.

* [threads] Fix async_abort_critical for full coop

If the foreign external thread doesn't have any managed methods on its
callstack, but it once called a native-to-managed wrapper, it will be left by
mono_threads_detach_coop in GC Safe (BLOCKING) state.  But under full coop, GC
Safe state is considered suspended, so mono_thread_info_safe_suspend_and_run
will run async_abort_critical for the thread.

But the thread may never call into Mono again, in which case it will never
safepoint and aknowledge the abort interruption.  So set thread_will_abort to
FALSE in this case, so that mono_thread_manage_internal won't try to wait for it.

---

Related to an issue first identified in https://github.com/mono/mono/pull/18517

---

This supersedes mono/mono#18656

Co-authored-by: lambdageek <lambdageek@users.noreply.github.com>