Delete dead code (#18783)

- Leftover from unobserved exception desktop quirk
- Unnecessary calls to AppDomain.IsFinalizingForUnload(). They were actually never required even with AppDomain support because of Environment.HasShutdownStarted returns true during AppDomain unload.

9 files changed, 17 insertions, 85 deletions

diff --git a/src/System.Private.CoreLib/src/System/AppDomain.cs b/src/System.Private.CoreLib/src/System/AppDomain.cs
index 292fa926cb..d96710b8e9 100644
--- a/src/System.Private.CoreLib/src/System/AppDomain.cs
+++ b/src/System.Private.CoreLib/src/System/AppDomain.cs
@@ -267,11 +267,6 @@ namespace System
             return nGetAssemblies(forIntrospection);
         }
 
-        // this is true when we've just started going through the finalizers and are forcing objects to finalize
-        // so must be aware that certain infrastructure may have gone away
-        [MethodImpl(MethodImplOptions.InternalCall)]
-        public extern bool IsFinalizingForUnload();
-
         [MethodImpl(MethodImplOptions.InternalCall)]
         internal static extern void PublishAnonymouslyHostedDynamicMethodsAssembly(RuntimeAssembly assemblyHandle);
 
diff --git a/src/System.Private.CoreLib/src/System/Reflection/Emit/DynamicILGenerator.cs b/src/System.Private.CoreLib/src/System/Reflection/Emit/DynamicILGenerator.cs
index 48ad93dbda..3315daf173 100644
--- a/src/System.Private.CoreLib/src/System/Reflection/Emit/DynamicILGenerator.cs
+++ b/src/System.Private.CoreLib/src/System/Reflection/Emit/DynamicILGenerator.cs
@@ -641,8 +641,7 @@ namespace System.Reflection.Emit
                 // We go over all DynamicMethodDesc during AppDomain shutdown and make sure
                 // that everything associated with them is released. So it is ok to skip reregistration
                 // for finalization during appdomain shutdown
-                if (!Environment.HasShutdownStarted &&
-                    !AppDomain.CurrentDomain.IsFinalizingForUnload())
+                if (!Environment.HasShutdownStarted)
                 {
                     // Try again later.
                     GC.ReRegisterForFinalize(this);
@@ -667,8 +666,7 @@ namespace System.Reflection.Emit
                 // It is not safe to destroy the method if the managed resolver is alive.
                 if (RuntimeMethodHandle.GetResolver(m_methodHandle) != null)
                 {
-                    if (!Environment.HasShutdownStarted &&
-                        !AppDomain.CurrentDomain.IsFinalizingForUnload())
+                    if (!Environment.HasShutdownStarted)
                     {
                         // Somebody might have been holding a reference on us via weak handle.
                         // We will keep trying. It will be hopefully released eventually.
diff --git a/src/System.Private.CoreLib/src/System/Reflection/LoaderAllocator.cs b/src/System.Private.CoreLib/src/System/Reflection/LoaderAllocator.cs
index 54477734c2..1119f07fa4 100644
--- a/src/System.Private.CoreLib/src/System/Reflection/LoaderAllocator.cs
+++ b/src/System.Private.CoreLib/src/System/Reflection/LoaderAllocator.cs
@@ -43,10 +43,9 @@ namespace System.Reflection
 
             // Assemblies and LoaderAllocators will be cleaned up during AppDomain shutdown in
             // unmanaged code
-            // So it is ok to skip reregistration and cleanup for finalization during appdomain shutdown.
+            // So it is ok to skip reregistration and cleanup for finalization during shutdown.
             // We also avoid early finalization of LoaderAllocatorScout due to AD unload when the object was inside DelayedFinalizationList.
-            if (!Environment.HasShutdownStarted &&
-                !AppDomain.CurrentDomain.IsFinalizingForUnload())
+            if (!Environment.HasShutdownStarted)
             {
                 // Destroy returns false if the managed LoaderAllocator is still alive.
                 if (!Destroy(m_nativeLoaderAllocator))
diff --git a/src/System.Private.CoreLib/src/System/Threading/Tasks/TaskExceptionHolder.cs b/src/System.Private.CoreLib/src/System/Threading/Tasks/TaskExceptionHolder.cs
index a96852a6f0..3046bd9db8 100644
--- a/src/System.Private.CoreLib/src/System/Threading/Tasks/TaskExceptionHolder.cs
+++ b/src/System.Private.CoreLib/src/System/Threading/Tasks/TaskExceptionHolder.cs
@@ -13,32 +13,23 @@
 // Disable the "reference to volatile field not treated as volatile" error.
 #pragma warning disable 0420
 
+using System.Collections.Generic;
+using System.Collections.ObjectModel;
+using System.Diagnostics;
+using System.Runtime.ExceptionServices;
+
 namespace System.Threading.Tasks
 {
-    using System;
-    using System.Collections.Generic;
-    using System.Collections.ObjectModel;
-    using System.Diagnostics;
-    using System.Runtime.ExceptionServices;
-    using System.Security;
-
     /// <summary>
     /// An exception holder manages a list of exceptions for one particular task.
     /// It offers the ability to aggregate, but more importantly, also offers intrinsic
     /// support for propagating unhandled exceptions that are never observed. It does
-    /// this by aggregating and throwing if the holder is ever GC'd without the holder's
-    /// contents ever having been requested (e.g. by a Task.Wait, Task.get_Exception, etc).
-    /// This behavior is prominent in .NET 4 but is suppressed by default beyond that release.
+    /// this by aggregating and calling UnobservedTaskException event event if the holder 
+    /// is ever GC'd without the holder's contents ever having been requested 
+    /// (e.g. by a Task.Wait, Task.get_Exception, etc).
     /// </summary>
     internal class TaskExceptionHolder
     {
-        /// <summary>Whether we should propagate exceptions on the finalizer.</summary>
-        private readonly static bool s_failFastOnUnobservedException = ShouldFailFastOnUnobservedException();
-        /// <summary>Whether the AppDomain has started to unload.</summary>
-        private static volatile bool s_domainUnloadStarted;
-        /// <summary>An event handler used to notify of domain unload.</summary>
-        private static volatile EventHandler s_adUnloadEventHandler;
-
         /// <summary>The task with which this holder is associated.</summary>
         private readonly Task m_task;
         /// <summary>
@@ -59,28 +50,6 @@ namespace System.Threading.Tasks
         {
             Debug.Assert(task != null, "Expected a non-null task.");
             m_task = task;
-            EnsureADUnloadCallbackRegistered();
-        }
-
-        private static bool ShouldFailFastOnUnobservedException()
-        {
-            return false;
-        }
-
-        private static void EnsureADUnloadCallbackRegistered()
-        {
-            if (s_adUnloadEventHandler == null &&
-                Interlocked.CompareExchange(ref s_adUnloadEventHandler,
-                                             AppDomainUnloadCallback,
-                                             null) == null)
-            {
-                AppDomain.CurrentDomain.DomainUnload += s_adUnloadEventHandler;
-            }
-        }
-
-        private static void AppDomainUnloadCallback(object sender, EventArgs e)
-        {
-            s_domainUnloadStarted = true;
         }
 
         /// <summary>
@@ -92,8 +61,7 @@ namespace System.Threading.Tasks
             // We need to do this filtering because all TaskExceptionHolders will be finalized during shutdown or unload
             // regardles of reachability of the task (i.e. even if the user code was about to observe the task's exception),
             // which can otherwise lead to spurious crashes during shutdown.
-            if (m_faultExceptions != null && !m_isHandled &&
-                !Environment.HasShutdownStarted && !AppDomain.CurrentDomain.IsFinalizingForUnload() && !s_domainUnloadStarted)
+            if (m_faultExceptions != null && !m_isHandled && !Environment.HasShutdownStarted)
             {
                 // We don't want to crash the finalizer thread if any ThreadAbortExceptions 
                 // occur in the list or in any nested AggregateExceptions.  
@@ -122,22 +90,12 @@ namespace System.Threading.Tasks
                 // other finalizer, and the Task was finalized before the holder, the holder
                 // will have been marked as handled before even getting here.
 
-                // Give users a chance to keep this exception from crashing the process
-
-                // First, publish the unobserved exception and allow users to observe it
+                // Publish the unobserved exception and allow users to observe it
                 AggregateException exceptionToThrow = new AggregateException(
                     SR.TaskExceptionHolder_UnhandledException,
                     m_faultExceptions);
                 UnobservedTaskExceptionEventArgs ueea = new UnobservedTaskExceptionEventArgs(exceptionToThrow);
                 TaskScheduler.PublishUnobservedTaskException(m_task, ueea);
-
-                // Now, if we are still unobserved and we're configured to crash on unobserved, throw the exception.
-                // We need to publish the event above even if we're not going to crash, hence
-                // why this check doesn't come at the beginning of the method.
-                if (s_failFastOnUnobservedException && !ueea.m_observed)
-                {
-                    throw exceptionToThrow;
-                }
             }
         }
 
diff --git a/src/System.Private.CoreLib/src/System/Threading/ThreadPool.cs b/src/System.Private.CoreLib/src/System/Threading/ThreadPool.cs
index 490c805729..d9b378a2ab 100644
--- a/src/System.Private.CoreLib/src/System/Threading/ThreadPool.cs
+++ b/src/System.Private.CoreLib/src/System/Threading/ThreadPool.cs
@@ -697,7 +697,7 @@ namespace System.Threading
             // if we're in the process of shutting down or unloading the AD.  In those cases, the work won't
             // execute anyway.  And there are subtle race conditions involved there that would lead us to do the wrong
             // thing anyway.  So we'll only clean up if this is a "normal" finalization.
-            if (!(Environment.HasShutdownStarted || AppDomain.CurrentDomain.IsFinalizingForUnload()))
+            if (!Environment.HasShutdownStarted)
                 CleanUp();
         }
     }
@@ -902,7 +902,7 @@ namespace System.Threading
         ~QueueUserWorkItemCallbackBase()
         {
             Debug.Assert(
-                executed != 0 || Environment.HasShutdownStarted || AppDomain.CurrentDomain.IsFinalizingForUnload(),
+                executed != 0 || Environment.HasShutdownStarted,
                 "A QueueUserWorkItemCallback was never called!");
         }
 
diff --git a/src/System.Private.CoreLib/src/System/Threading/Timer.cs b/src/System.Private.CoreLib/src/System/Threading/Timer.cs
index d9aa01e505..b7e0cfb10e 100644
--- a/src/System.Private.CoreLib/src/System/Threading/Timer.cs
+++ b/src/System.Private.CoreLib/src/System/Threading/Timer.cs
@@ -650,7 +650,7 @@ namespace System.Threading
             // Note that in either case, the Timer still won't fire, because ThreadPool threads won't be
             // allowed to run in this AppDomain.
             //
-            if (Environment.HasShutdownStarted || AppDomain.CurrentDomain.IsFinalizingForUnload())
+            if (Environment.HasShutdownStarted)
                 return;
 
             m_timer.Close();
diff --git a/src/vm/appdomainnative.cpp b/src/vm/appdomainnative.cpp
index 3c1b64a370..49e0f62013 100644
--- a/src/vm/appdomainnative.cpp
+++ b/src/vm/appdomainnative.cpp
@@ -419,22 +419,6 @@ FCIMPL1(Object*, AppDomainNative::GetDynamicDir, AppDomainBaseObject* refThisUNS
 }
 FCIMPLEND
 
-FCIMPL1(FC_BOOL_RET, AppDomainNative::IsFinalizingForUnload, AppDomainBaseObject* refThisUNSAFE)
-{
-    FCALL_CONTRACT;
-
-    BOOL            retVal = FALSE;
-    APPDOMAINREF    refThis = (APPDOMAINREF) refThisUNSAFE;
-    HELPER_METHOD_FRAME_BEGIN_RET_1(refThis);
-
-    AppDomain* pApp = ValidateArg(refThis);
-    retVal = pApp->IsFinalizing();
-
-    HELPER_METHOD_FRAME_END();
-    FC_RETURN_BOOL(retVal);
-}
-FCIMPLEND
-
 FCIMPL2(StringObject*, AppDomainNative::nApplyPolicy, AppDomainBaseObject* refThisUNSAFE, AssemblyNameBaseObject* refAssemblyNameUNSAFE)
 {
     FCALL_CONTRACT;
diff --git a/src/vm/appdomainnative.hpp b/src/vm/appdomainnative.hpp
index e2d564a747..9728ef8ec0 100644
--- a/src/vm/appdomainnative.hpp
+++ b/src/vm/appdomainnative.hpp
@@ -33,7 +33,6 @@ public:
     static FCDECL1(INT32, GetId, AppDomainBaseObject* refThisUNSAFE);
     static FCDECL1(INT32, GetIdForUnload, AppDomainBaseObject* refDomainUNSAFE);
     static FCDECL1(FC_BOOL_RET, IsDomainIdValid, INT32 dwId);
-    static FCDECL1(FC_BOOL_RET, IsFinalizingForUnload, AppDomainBaseObject* refThisUNSAFE);
     static FCDECL1(void, ForceToSharedDomain, Object* pObjectUNSAFE);
     static FCDECL1(LPVOID,  GetFusionContext, AppDomainBaseObject* refThis);
     static FCDECL2(Object*, IsStringInterned, AppDomainBaseObject* refThis, StringObject* pString);
diff --git a/src/vm/ecalllist.h b/src/vm/ecalllist.h
index e575dedfb0..8ee51b455b 100644
--- a/src/vm/ecalllist.h
+++ b/src/vm/ecalllist.h
@@ -461,7 +461,6 @@ FCFuncStart(gAppDomainFuncs)
     FCFuncElement("GetOrInternString", AppDomainNative::GetOrInternString)
     QCFuncElement("nSetupBindingPaths", AppDomainNative::SetupBindingPaths)
     QCFuncElement("nSetNativeDllSearchDirectories", AppDomainNative::SetNativeDllSearchDirectories)
-    FCFuncElement("IsFinalizingForUnload", AppDomainNative::IsFinalizingForUnload)
     FCFuncElement("PublishAnonymouslyHostedDynamicMethodsAssembly", AppDomainNative::PublishAnonymouslyHostedDynamicMethodsAssembly)
 #ifdef FEATURE_APPDOMAIN_RESOURCE_MONITORING
     FCFuncElement("nEnableMonitoring", AppDomainNative::EnableMonitoring)