From 65bcecebde4aebd5926952ccfc4dc8e7fbf7eaed Mon Sep 17 00:00:00 2001
From: Johan Lorensson <lateralusx.github@gmail.com>
Date: Wed, 21 Apr 2021 09:22:32 +0200
Subject: [PATCH] Lazy init of ThreadWaitInfo in Thread.Mono.cs to prevent
 crash on external attached threads. (#51483)

* Init ThreadWaitInfo to prevent crash on external attached threads.

Commit https://github.com/dotnet/runtime/commit/457f58cd30082c9de0be9ca62e64fa179c37dab3
added a new managed object into Mono's managed thread object, _waitInfo.

This new object was initialized in Thread managed constructor, problem
with that is for all threads that gets their thread object created in
native code, create_thread_object, that code did the same thing as
constructor before commit, since managed constructor called down into
native code calling the same native method as create_thread_object meaning
a thread object created from managed or native would be initialized
identical.

After above commit this is no longer true, meaning that all managed thread
objects created in native code (like attached external threads) won't
get _waitInfo initialized and that leads to a crash when OnThreadExiting
gets called from Finalizer thread.

Fix is to make sure _waitInfo get initialize regardless of thread object
gets created from managed or native code.

This fix implements a lazy init as part of property access, I tested a
native version of the fix, calling back into managed to init managed
parts of thread when tread gets created in native, but turns out that
such a fix gets a little complicated, since some of the threads are
created early when it's not possible to run managed code, so that in turn
needed additional changes into the init order, that in turn has bigger
impact and could cause other unwanted side effects.

Fixing it in managed using lazy/on demand init is simpler/cleaner approach
but might come with a small performance hit for callers of WaitInfo property
(will add volatile read and conditional branch), but looking at callers
of the property, they appear in functions with rather high "cost",
like Sleep, NewMutex, Wait, SignalAndWait, Interrupt and
RelinquishOwnership.
---
 .../src/System/Threading/Thread.Mono.cs       | 32 ++++++++-----------
 1 file changed, 14 insertions(+), 18 deletions(-)

diff --git a/src/mono/System.Private.CoreLib/src/System/Threading/Thread.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Threading/Thread.Mono.cs
index 96a7333de3434..c9cd0ed68eb17 100644
--- a/src/mono/System.Private.CoreLib/src/System/Threading/Thread.Mono.cs
+++ b/src/mono/System.Private.CoreLib/src/System/Threading/Thread.Mono.cs
@@ -66,7 +66,7 @@ public partial class Thread
         internal ExecutionContext? _executionContext;
         internal SynchronizationContext? _synchronizationContext;
 #if TARGET_UNIX || TARGET_BROWSER
-        internal WaitSubsystem.ThreadWaitInfo _waitInfo;
+        internal WaitSubsystem.ThreadWaitInfo? _waitInfo;
 #endif
 
         // This is used for a quick check on thread pool threads after running a work item to determine if the name, background
@@ -138,9 +138,20 @@ internal static int OptimalMaxSpinWaitsPerSpinIteration
                 return 7;
             }
         }
-
 #if TARGET_UNIX || TARGET_BROWSER
-        internal WaitSubsystem.ThreadWaitInfo WaitInfo => _waitInfo;
+        internal WaitSubsystem.ThreadWaitInfo WaitInfo
+        {
+            get
+            {
+                return Volatile.Read(ref _waitInfo) ?? AllocateWaitInfo();
+
+                WaitSubsystem.ThreadWaitInfo AllocateWaitInfo()
+                {
+                    Interlocked.CompareExchange(ref _waitInfo, new WaitSubsystem.ThreadWaitInfo(this), null!);
+                    return _waitInfo;
+                }
+            }
+        }
 #endif
 
         public ThreadPriority Priority
@@ -199,25 +210,13 @@ public bool Join(int millisecondsTimeout)
         }
 
 #if TARGET_UNIX || TARGET_BROWSER
-        [MemberNotNull(nameof(_waitInfo))]
         [DynamicDependency(nameof(OnThreadExiting))]
 #endif
         private void Initialize()
         {
             InitInternal(this);
-#if TARGET_UNIX || TARGET_BROWSER
-            _waitInfo = new WaitSubsystem.ThreadWaitInfo(this);
-#endif
         }
 
-#if TARGET_UNIX || TARGET_BROWSER
-        private void EnsureWaitInfo()
-        {
-            if (_waitInfo == null)
-                _waitInfo = new WaitSubsystem.ThreadWaitInfo(this);
-        }
-#endif
-
         public static void SpinWait(int iterations)
         {
             if (iterations < 0)
@@ -321,9 +320,6 @@ private static void OnThreadExiting(Thread thread)
         private static Thread InitializeCurrentThread()
         {
             var current = GetCurrentThread();
-#if TARGET_UNIX || TARGET_BROWSER
-            current.EnsureWaitInfo();
-#endif
             t_currentThread = current;
             return t_currentThread;
         }