[Local GC] Move some EE-specific finalize-on-unload logic out of the GC (#10598)

* Move some EE-specific logic to a callback on GCToEEInterface

* Fix the sample

* Consistent style for pointers

* Code review feedback: Move app domain index check into EE callback and make ShouldFinalizeObjectForUnload always return true (due to CoreCLR not having app domains)

* Code review feedback: Fix a comment and add a TODO for bringing these changes to desktop

7 files changed, 32 insertions, 33 deletions

diff --git a/src/gc/env/gcenv.ee.h b/src/gc/env/gcenv.ee.h
index 689b1cc6e1..8c4dfccce9 100644
--- a/src/gc/env/gcenv.ee.h
+++ b/src/gc/env/gcenv.ee.h
@@ -70,6 +70,7 @@ public:
     static void EnableFinalization(bool foundFinalizers);
 
     static void HandleFatalError(unsigned int exitCode);
+    static bool ShouldFinalizeObjectForUnload(AppDomain* pDomain, Object* obj);
 };
 
 #endif // __GCENV_EE_H__
diff --git a/src/gc/gc.cpp b/src/gc/gc.cpp
index d8a59cd5e4..ab8e56bc8a 100644
--- a/src/gc/gc.cpp
+++ b/src/gc/gc.cpp
@@ -36102,43 +36102,15 @@ CFinalize::FinalizeSegForAppDomain (AppDomain *pDomain,
         // if it has the index we are looking for. If the methodtable is null, it can't be from the
         // unloading domain, so skip it.
         if (method_table(obj) == NULL)
+        {
             continue;
+        }
 
-        // eagerly finalize all objects except those that may be agile.
-        if (obj->GetAppDomainIndex() != pDomain->GetIndex())
+        // does the EE actually want us to finalize this object?
+        if (!GCToEEInterface::ShouldFinalizeObjectForUnload(pDomain, obj))
+        {
             continue;
-
-#ifndef FEATURE_REDHAWK
-        if (method_table(obj)->IsAgileAndFinalizable())
-        {
-            // If an object is both agile & finalizable, we leave it in the
-            // finalization queue during unload.  This is OK, since it's agile.
-            // Right now only threads can be this way, so if that ever changes, change
-            // the assert to just continue if not a thread.
-            _ASSERTE(method_table(obj) == g_pThreadClass);
-
-            if (method_table(obj) == g_pThreadClass)
-            {
-                // However, an unstarted thread should be finalized. It could be holding a delegate
-                // in the domain we want to unload. Once the thread has been started, its
-                // delegate is cleared so only unstarted threads are a problem.
-                Thread *pThread = ((THREADBASEREF)ObjectToOBJECTREF(obj))->GetInternal();
-                if (! pThread || ! pThread->IsUnstarted())
-                {
-                    // This appdomain is going to be gone soon so let us assign
-                    // it the appdomain that's guaranteed to exist
-                    // The object is agile and the delegate should be null so we can do it
-                    obj->GetHeader()->ResetAppDomainIndexNoFailure(SystemDomain::System()->DefaultDomain()->GetIndex());
-                    continue;
-                }
-            }
-            else
-            {
-                obj->GetHeader()->ResetAppDomainIndexNoFailure(SystemDomain::System()->DefaultDomain()->GetIndex());
-                continue;
-            }
         }
-#endif //!FEATURE_REDHAWK
 
         if (!fRunFinalizers || (obj->GetHeader()->GetBits()) & BIT_SBLK_FINALIZER_RUN)
         {
diff --git a/src/gc/gcenv.ee.standalone.inl b/src/gc/gcenv.ee.standalone.inl
index c391ef8126..328acef056 100644
--- a/src/gc/gcenv.ee.standalone.inl
+++ b/src/gc/gcenv.ee.standalone.inl
@@ -213,6 +213,12 @@ ALWAYS_INLINE void GCToEEInterface::HandleFatalError(unsigned int exitCode)
     g_theGCToCLR->HandleFatalError(exitCode);
 }
 
+ALWAYS_INLINE bool GCToEEInterface::ShouldFinalizeObjectForUnload(AppDomain* pDomain, Object* obj)
+{
+    assert(g_theGCToCLR != nullptr);
+    return g_theGCToCLR->ShouldFinalizeObjectForUnload(pDomain, obj);
+}
+
 #undef ALWAYS_INLINE
 
 #endif // __GCTOENV_EE_STANDALONE_INL__
diff --git a/src/gc/gcinterface.ee.h b/src/gc/gcinterface.ee.h
index da74665b69..5c595b417f 100644
--- a/src/gc/gcinterface.ee.h
+++ b/src/gc/gcinterface.ee.h
@@ -137,6 +137,11 @@ public:
     // Signals to the EE that the GC encountered a fatal error and can't recover.
     virtual
     void HandleFatalError(unsigned int exitCode) = 0;
+
+    // Asks the EE if it wants a particular object to be finalized when unloading
+    // an app domain.
+    virtual
+    bool ShouldFinalizeObjectForUnload(AppDomain* pDomain, Object* obj) = 0;
 };
 
 #endif // _GCINTERFACE_EE_H_
diff --git a/src/gc/sample/gcenv.ee.cpp b/src/gc/sample/gcenv.ee.cpp
index 07be244375..aaca51e8b8 100644
--- a/src/gc/sample/gcenv.ee.cpp
+++ b/src/gc/sample/gcenv.ee.cpp
@@ -270,6 +270,11 @@ void GCToEEInterface::HandleFatalError(unsigned int exitCode)
     abort();
 }
 
+bool GCToEEInterface::ShouldFinalizeObjectForUnload(AppDomain* pDomain, Object* obj)
+{
+    return true;
+}
+
 bool IsGCSpecialThread()
 {
     // TODO: Implement for background GC
diff --git a/src/vm/gcenv.ee.cpp b/src/vm/gcenv.ee.cpp
index cf2728d6cb..f3c139e66d 100644
--- a/src/vm/gcenv.ee.cpp
+++ b/src/vm/gcenv.ee.cpp
@@ -1350,3 +1350,12 @@ void GCToEEInterface::HandleFatalError(unsigned int exitCode)
 {
     EEPOLICY_HANDLE_FATAL_ERROR(exitCode);
 }
+
+bool GCToEEInterface::ShouldFinalizeObjectForUnload(AppDomain* pDomain, Object* obj)
+{
+    // CoreCLR does not have appdomains, so this code path is dead. Other runtimes may
+    // choose to inspect the object being finalized here.
+    // [DESKTOP TODO] Desktop looks for "agile and finalizable" objects and may choose
+    // to move them to a new app domain instead of finalizing them here.
+    return true;
+}
diff --git a/src/vm/gcenv.ee.h b/src/vm/gcenv.ee.h
index e3ced6c65a..030eb71807 100644
--- a/src/vm/gcenv.ee.h
+++ b/src/vm/gcenv.ee.h
@@ -45,6 +45,7 @@ public:
 
     void EnableFinalization(bool foundFinalizers);
     void HandleFatalError(unsigned int exitCode);
+    bool ShouldFinalizeObjectForUnload(AppDomain* pDomain, Object* obj);
 };
 
 #endif // FEATURE_STANDALONE_GC