Clean up the thread local memory regardless of managed thread's presence

[kunalspathak]

Turns out that #95362 doesn't fully solve the problem of memory leak before we were freeing the thread local memory only if associated managed thread existed, which most of the time won't be around because we would reset it much earlier. To fix, we should delete the thread local memory regardless of underlying managed thread is present.

Below is the sample C# code that prints thread ids [os, managed] and the before/after logs of instrumentation I added.

repro code

public class A { [ThreadStatic] public static string a; }
public class B { [ThreadStatic] public static string a; }
public class C { [ThreadStatic] public static string a; }
public class D { [ThreadStatic] public static string a; }
public class E { [ThreadStatic] public static string a; }
public class F { [ThreadStatic] public static string a; }
public class G { [ThreadStatic] public static string a; }
public class H { [ThreadStatic] public static int a; }
static void Main()
{
   const int threadCount = 10;
   Thread[] threads = new Thread[threadCount];
   for (int i = 0; i < threadCount; i++)
   {
       threads[i] = new Thread(new ThreadStart(Work));
       threads[i].Start();
   }

   for (int i = 0; i < threadCount; i++)
   {
       threads[i].Join();
   }
   
   // give enough time to make sure the thread cleanup is called
   for (int i = 0; i < threadCount; i++)
   {
       Work2();
   }
}

static void Work2()
{
   for (int i = 0; i < 1000; i++)
   {
       A.a += "a" + i;
       B.a += "a" + i;
       C.a += "a" + i;
       D.a += "a" + i;
       E.a += "a" + i;
       F.a += "a" + i;
       G.a += "a" + i;
       H.a += i;
   }
}

static void Work()
{
   for (int i = 0; i < 10; i++)
   {
       A.a = "a" + i;
       B.a = "a" + i;
       C.a = "a" + i;
       D.a = "a" + i;
       E.a = "a" + i;
       F.a = "a" + i;
       G.a = "a" + i;
       H.a = i;
   }

   Console.WriteLine($"Done with [{GetCurrentThreadId()}, {Thread.CurrentThread.ManagedThreadId}]");
}

// Importing the GetCurrentThreadId function from Kernel32.dll
[DllImport("Kernel32.dll")]
public static extern uint GetCurrentThreadId();

Before logs

[71260] In EnsureTlsDestructionMonitor()
[62564] In EnsureTlsDestructionMonitor()
[15128] In EnsureTlsDestructionMonitor()
[68936] In EnsureTlsDestructionMonitor()
[12792] In EnsureTlsDestructionMonitor()
[11820] In EnsureTlsDestructionMonitor()
[47812] In EnsureTlsDestructionMonitor()
[54956] In EnsureTlsDestructionMonitor()
[18884] In EnsureTlsDestructionMonitor()
[32832] In EnsureTlsDestructionMonitor()
[63360] In EnsureTlsDestructionMonitor()
[15144] In EnsureTlsDestructionMonitor()
Done with [68936, 4]
Done with [18884, 9]
[68936, 4] SetThread to null
[68936] In ~TlsDestructionMonitor()
Done with [54956, 8]
[18884, 9] SetThread to null
[18884] In ~TlsDestructionMonitor()
Done with [12792, 5]
[54956, 8] SetThread to null
[54956] In ~TlsDestructionMonitor()
Done with [32832, 10]
[12792, 5] SetThread to null
[32832, 10] SetThread to null
Done with [15144, 12]
Done with [11820, 6]
[12792] In ~TlsDestructionMonitor()
Done with [15128, 3]
[32832] In ~TlsDestructionMonitor()
Done with [47812, 7]
Done with [63360, 11]
[15144, 12] SetThread to null
[11820, 6] SetThread to null
[15144] In ~TlsDestructionMonitor()
[15128, 3] SetThread to null
[11820] In ~TlsDestructionMonitor()
[47812, 7] SetThread to null
[63360, 11] SetThread to null
[15128] In ~TlsDestructionMonitor()
[47812] In ~TlsDestructionMonitor()
[63360] In ~TlsDestructionMonitor()
[71260] In ~TlsDestructionMonitor()
[71260, 1] SetThread to null
[71260, 1] Calling DeleteThreadLocalMemory
[71260] DeleteThreadLocalMemory
[71260] Deleting for non-gc
[71260] Deleting for gc

After logs

[38692] In EnsureTlsDestructionMonitor()
[51888] In EnsureTlsDestructionMonitor()
[75516] In EnsureTlsDestructionMonitor()
[56776] In EnsureTlsDestructionMonitor()
[59300] In EnsureTlsDestructionMonitor()
[38740] In EnsureTlsDestructionMonitor()
[71528] In EnsureTlsDestructionMonitor()
[61632] In EnsureTlsDestructionMonitor()
[42288] In EnsureTlsDestructionMonitor()
[53784] In EnsureTlsDestructionMonitor()
[46500] In EnsureTlsDestructionMonitor()
[62232] In EnsureTlsDestructionMonitor()
Done with [46500, 11]
Done with [75516, 3]
[46500, 11] SetThread to null
[75516, 3] SetThread to null
Done with [61632, 8]
[46500] In ~TlsDestructionMonitor()
Done with [62232, 12]
[46500, NULL] Calling DeleteThreadLocalMemory
[46500] DeleteThreadLocalMemory
[46500] Deleting for non-gc
Done with [42288, 9]
[46500] Deleting for gc
[61632, 8] SetThread to null
Done with [59300, 5]
[62232, 12] SetThread to null
[42288, 9] SetThread to null
Done with [56776, 4]
[75516] In ~TlsDestructionMonitor()
Done with [38740, 6]
[75516, NULL] Calling DeleteThreadLocalMemory
[75516] DeleteThreadLocalMemory
Done with [53784, 10]
[75516] Deleting for non-gc
Done with [71528, 7]
[75516] Deleting for gc
[59300, 5] SetThread to null
[61632] In ~TlsDestructionMonitor()
[61632, NULL] Calling DeleteThreadLocalMemory
[61632] DeleteThreadLocalMemory
[61632] Deleting for non-gc
[61632] Deleting for gc
[56776, 4] SetThread to null
[62232] In ~TlsDestructionMonitor()
[62232, NULL] Calling DeleteThreadLocalMemory
[62232] DeleteThreadLocalMemory
[62232] Deleting for non-gc
[62232] Deleting for gc
[38740, 6] SetThread to null
[42288] In ~TlsDestructionMonitor()
[42288, NULL] Calling DeleteThreadLocalMemory
[42288] DeleteThreadLocalMemory
[42288] Deleting for non-gc
[42288] Deleting for gc
[53784, 10] SetThread to null
[59300] In ~TlsDestructionMonitor()
[59300, NULL] Calling DeleteThreadLocalMemory
[59300] DeleteThreadLocalMemory
[59300] Deleting for non-gc
[59300] Deleting for gc
[71528, 7] SetThread to null
[56776] In ~TlsDestructionMonitor()
[56776, NULL] Calling DeleteThreadLocalMemory
[56776] DeleteThreadLocalMemory
[56776] Deleting for non-gc
[56776] Deleting for gc
[38740] In ~TlsDestructionMonitor()
[38740, NULL] Calling DeleteThreadLocalMemory
[38740] DeleteThreadLocalMemory
[38740] Deleting for non-gc
[38740] Deleting for gc
[53784] In ~TlsDestructionMonitor()
[53784, NULL] Calling DeleteThreadLocalMemory
[53784] DeleteThreadLocalMemory
[53784] Deleting for non-gc
[53784] Deleting for gc
[71528] In ~TlsDestructionMonitor()
[71528, NULL] Calling DeleteThreadLocalMemory
[71528] DeleteThreadLocalMemory
[71528] Deleting for non-gc
[71528] Deleting for gc
[38692] In ~TlsDestructionMonitor()
[38692, 1] SetThread to null
[38692, 1] Calling DeleteThreadLocalMemory
[38692] DeleteThreadLocalMemory
[38692] Deleting for non-gc
[38692] Deleting for gc

Logging changes

diff --git a/src/coreclr/vm/ceemain.cpp b/src/coreclr/vm/ceemain.cpp
index 1c58c941fb2..9213b128dc6 100644
--- a/src/coreclr/vm/ceemain.cpp
+++ b/src/coreclr/vm/ceemain.cpp
@@ -1731,6 +1731,8 @@ struct TlsDestructionMonitor
     {
         if (m_activated)
         {
+            printf("[%u] In ~TlsDestructionMonitor()\n", GetCurrentThreadId());
+
             Thread* thread = GetThreadNULLOk();
             if (thread)
             {
@@ -1752,11 +1754,16 @@ struct TlsDestructionMonitor
                     GCX_COOP_NO_DTOR_END();
                 }
                 thread->DetachThread(TRUE);
+                printf("[%u, %u] Calling DeleteThreadLocalMemory\n", GetCurrentThreadId(), thread->m_ThreadId);
                 DeleteThreadLocalMemory();
             }
 
             ThreadDetaching();
         }
+        else
+        {
+            printf("[%u] -- not activated\n", GetCurrentThreadId());
+        }
     }
 };
 
@@ -1766,6 +1773,7 @@ thread_local TlsDestructionMonitor tls_destructionMonitor;
 
 void EnsureTlsDestructionMonitor()
 {
+    printf("[%u] In EnsureTlsDestructionMonitor()\n", GetCurrentThreadId());
     tls_destructionMonitor.Activate();
 }
 
@@ -1786,13 +1794,16 @@ void DeleteThreadLocalMemory()
     t_ThreadStatics.NonGCMaxThreadStaticBlocks = 0;
     t_ThreadStatics.GCMaxThreadStaticBlocks = 0;
 
+    printf("[%u] DeleteThreadLocalMemory\n", GetCurrentThreadId());
     if (t_ThreadStatics.NonGCThreadStaticBlocks != nullptr)
     {
+        printf("[%u] Deleting for non-gc\n", GetCurrentThreadId());
         delete[] t_ThreadStatics.NonGCThreadStaticBlocks;
         t_ThreadStatics.NonGCThreadStaticBlocks = nullptr;
     }
     if (t_ThreadStatics.GCThreadStaticBlocks != nullptr)
     {
+        printf("[%u] Deleting for gc\n", GetCurrentThreadId());
         delete[] t_ThreadStatics.GCThreadStaticBlocks;
         t_ThreadStatics.GCThreadStaticBlocks = nullptr;
     }
diff --git a/src/coreclr/vm/threads.cpp b/src/coreclr/vm/threads.cpp
index edbd6a011f3..db1c6360a41 100644
--- a/src/coreclr/vm/threads.cpp
+++ b/src/coreclr/vm/threads.cpp
@@ -366,6 +366,10 @@ void SetThread(Thread* t)
 {
     LIMITED_METHOD_CONTRACT
 
+    if (gCurrentThreadInfo.m_pThread != NULL && t == NULL)
+    {
+        printf("[%u, %u] SetThread to null\n", GetCurrentThreadId(), gCurrentThreadInfo.m_pThread->m_ThreadId);
+    }
     gCurrentThreadInfo.m_pThread = t;
     if (t != NULL)
     {

[jkotas]

Thanks

[kunalspathak]

failures seems to be known issues.

[kunalspathak]

/backport to release/8.0-staging

[github-actions]

Started backporting to release/8.0-staging: https://github.com/dotnet/runtime/actions/runs/7124718987