diff options
author | Jan Kotas <jkotas@microsoft.com> | 2018-07-05 08:04:02 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-07-05 08:04:02 +0200 |
commit | cd1232d47cc028ebf6d22621ce63903a7e5c0e94 (patch) | |
tree | d2a3e6bccac62ce54a0ce13ef4eb16555f9eec50 | |
parent | 9f9a37d04460559c3f91927f1297c8195411a0a3 (diff) | |
download | coreclr-cd1232d47cc028ebf6d22621ce63903a7e5c0e94.tar.gz coreclr-cd1232d47cc028ebf6d22621ce63903a7e5c0e94.tar.bz2 coreclr-cd1232d47cc028ebf6d22621ce63903a7e5c0e94.zip |
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) |