Add better portable PDB caching to System.Diagnostics.StackTrace. (#17804) (#17842)

Add portable PDB caching to StackTrace.

This is the mscorlib side of the change.

4 files changed, 71 insertions, 83 deletions

diff --git a/src/mscorlib/src/System/Diagnostics/Stackframe.cs b/src/mscorlib/src/System/Diagnostics/Stackframe.cs
index f45de5cab9..884188d071 100644
--- a/src/mscorlib/src/System/Diagnostics/Stackframe.cs
+++ b/src/mscorlib/src/System/Diagnostics/Stackframe.cs
@@ -260,25 +260,24 @@ namespace System.Diagnostics
 
         private void BuildStackFrame(int skipFrames, bool fNeedFileInfo)
         {
-            using (StackFrameHelper StackF = new StackFrameHelper(null))
-            {
-                StackF.InitializeSourceInfo(0, fNeedFileInfo, null);
+            StackFrameHelper StackF = new StackFrameHelper(null);
+
+            StackF.InitializeSourceInfo(0, fNeedFileInfo, null);
 
-                int iNumOfFrames = StackF.GetNumberOfFrames();
+            int iNumOfFrames = StackF.GetNumberOfFrames();
 
-                skipFrames += StackTrace.CalculateFramesToSkip(StackF, iNumOfFrames);
+            skipFrames += StackTrace.CalculateFramesToSkip(StackF, iNumOfFrames);
 
-                if ((iNumOfFrames - skipFrames) > 0)
+            if ((iNumOfFrames - skipFrames) > 0)
+            {
+                method = StackF.GetMethodBase(skipFrames);
+                offset = StackF.GetOffset(skipFrames);
+                ILOffset = StackF.GetILOffset(skipFrames);
+                if (fNeedFileInfo)
                 {
-                    method = StackF.GetMethodBase(skipFrames);
-                    offset = StackF.GetOffset(skipFrames);
-                    ILOffset = StackF.GetILOffset(skipFrames);
-                    if (fNeedFileInfo)
-                    {
-                        strFileName = StackF.GetFilename(skipFrames);
-                        iLineNumber = StackF.GetLineNumber(skipFrames);
-                        iColumnNumber = StackF.GetColumnNumber(skipFrames);
-                    }
+                    strFileName = StackF.GetFilename(skipFrames);
+                    iLineNumber = StackF.GetLineNumber(skipFrames);
+                    iColumnNumber = StackF.GetColumnNumber(skipFrames);
                 }
             }
         }
diff --git a/src/mscorlib/src/System/Diagnostics/Stacktrace.cs b/src/mscorlib/src/System/Diagnostics/Stacktrace.cs
index aae2faa709..0535f02049 100644
--- a/src/mscorlib/src/System/Diagnostics/Stacktrace.cs
+++ b/src/mscorlib/src/System/Diagnostics/Stacktrace.cs
@@ -21,7 +21,7 @@ namespace System.Diagnostics
     // Modifying the order or fields of this object may require other changes 
     // to the unmanaged definition of the StackFrameHelper class, in 
     // VM\DebugDebugger.h. The binder will catch some of these layout problems.
-    internal class StackFrameHelper : IDisposable
+    internal class StackFrameHelper
     {
         private Thread targetThread;
         private int[] rgiOffset;
@@ -44,7 +44,6 @@ namespace System.Diagnostics
         private int[] rgiLineNumber;
         private int[] rgiColumnNumber;
         private bool[] rgiLastFrameFromForeignExceptionStackTrace;
-        private GetSourceLineInfoDelegate getSourceLineInfo;
         private int iFrameCount;
 #pragma warning restore 414
 
@@ -52,8 +51,7 @@ namespace System.Diagnostics
             IntPtr inMemoryPdbAddress, int inMemoryPdbSize, int methodToken, int ilOffset,
             out string sourceFile, out int sourceLine, out int sourceColumn);
 
-        private static Type s_symbolsType = null;
-        private static MethodInfo s_symbolsMethodInfo = null;
+        private static GetSourceLineInfoDelegate s_getSourceLineInfo = null;
 
         [ThreadStatic]
         private static int t_reentrancy = 0;
@@ -74,7 +72,6 @@ namespace System.Diagnostics
             rgFilename = null;
             rgiLineNumber = null;
             rgiColumnNumber = null;
-            getSourceLineInfo = null;
 
             rgiLastFrameFromForeignExceptionStackTrace = null;
 
@@ -107,27 +104,32 @@ namespace System.Diagnostics
             t_reentrancy++;
             try
             {
-                if (s_symbolsMethodInfo == null)
+                if (s_getSourceLineInfo == null)
                 {
-                    s_symbolsType = Type.GetType(
+                    Type symbolsType = Type.GetType(
                         "System.Diagnostics.StackTraceSymbols, System.Diagnostics.StackTrace, Version=4.0.1.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a",
                         throwOnError: false);
 
-                    if (s_symbolsType == null)
+                    if (symbolsType == null)
+                    {
                         return;
+                    }
 
-                    s_symbolsMethodInfo = s_symbolsType.GetMethod("GetSourceLineInfo", BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Instance);
-                    if (s_symbolsMethodInfo == null)
+                    MethodInfo symbolsMethodInfo = symbolsType.GetMethod("GetSourceLineInfo", BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Instance);
+                    if (symbolsMethodInfo == null)
+                    {
                         return;
-                }
+                    }
 
-                if (getSourceLineInfo == null)
-                {
                     // Create an instance of System.Diagnostics.Stacktrace.Symbols
-                    object target = Activator.CreateInstance(s_symbolsType);
+                    object target = Activator.CreateInstance(symbolsType);
 
                     // Create an instance delegate for the GetSourceLineInfo method
-                    getSourceLineInfo = (GetSourceLineInfoDelegate)s_symbolsMethodInfo.CreateDelegate(typeof(GetSourceLineInfoDelegate), target);
+                    GetSourceLineInfoDelegate getSourceLineInfo = (GetSourceLineInfoDelegate)symbolsMethodInfo.CreateDelegate(typeof(GetSourceLineInfoDelegate), target);
+
+                    // We could race with another thread. It doesn't matter if we win or lose, the losing instance will be GC'ed and all threads including this one will
+                    // use the winning instance
+                    Interlocked.CompareExchange(ref s_getSourceLineInfo, getSourceLineInfo, null);
                 }
 
                 for (int index = 0; index < iFrameCount; index++)
@@ -136,7 +138,7 @@ namespace System.Diagnostics
                     // ENC or the source/line info was already retrieved, the method token is 0.
                     if (rgiMethodToken[index] != 0)
                     {
-                        getSourceLineInfo(rgAssemblyPath[index], rgLoadedPeAddress[index], rgiLoadedPeSize[index],
+                        s_getSourceLineInfo(rgAssemblyPath[index], rgLoadedPeAddress[index], rgiLoadedPeSize[index],
                             rgInMemoryPdbAddress[index], rgiInMemoryPdbSize[index], rgiMethodToken[index],
                             rgiILOffset[index], out rgFilename[index], out rgiLineNumber[index], out rgiColumnNumber[index]);
                     }
@@ -151,18 +153,6 @@ namespace System.Diagnostics
             }
         }
 
-        void IDisposable.Dispose()
-        {
-            if (getSourceLineInfo != null)
-            {
-                IDisposable disposable = getSourceLineInfo.Target as IDisposable;
-                if (disposable != null)
-                {
-                    disposable.Dispose();
-                }
-            }
-        }
-
         public virtual MethodBase GetMethodBase(int i)
         {
             // There may be a better way to do this.
@@ -362,56 +352,57 @@ namespace System.Diagnostics
         {
             m_iMethodsToSkip += iSkip;
 
-            using (StackFrameHelper StackF = new StackFrameHelper(targetThread))
-            {
-                StackF.InitializeSourceInfo(0, fNeedFileInfo, e);
-
-                m_iNumOfFrames = StackF.GetNumberOfFrames();
+            StackFrameHelper StackF = new StackFrameHelper(targetThread);
+            
+            StackF.InitializeSourceInfo(0, fNeedFileInfo, e);
 
-                if (m_iMethodsToSkip > m_iNumOfFrames)
-                    m_iMethodsToSkip = m_iNumOfFrames;
+            m_iNumOfFrames = StackF.GetNumberOfFrames();
 
-                if (m_iNumOfFrames != 0)
-                {
-                    frames = new StackFrame[m_iNumOfFrames];
+            if (m_iMethodsToSkip > m_iNumOfFrames)
+                m_iMethodsToSkip = m_iNumOfFrames;
 
-                    for (int i = 0; i < m_iNumOfFrames; i++)
-                    {
-                        bool fDummy1 = true;
-                        bool fDummy2 = true;
-                        StackFrame sfTemp = new StackFrame(fDummy1, fDummy2);
+            if (m_iNumOfFrames != 0)
+            {
+                frames = new StackFrame[m_iNumOfFrames];
 
-                        sfTemp.SetMethodBase(StackF.GetMethodBase(i));
-                        sfTemp.SetOffset(StackF.GetOffset(i));
-                        sfTemp.SetILOffset(StackF.GetILOffset(i));
+                for (int i = 0; i < m_iNumOfFrames; i++)
+                {
+                    bool fDummy1 = true;
+                    bool fDummy2 = true;
+                    StackFrame sfTemp = new StackFrame(fDummy1, fDummy2);
 
-                        sfTemp.SetIsLastFrameFromForeignExceptionStackTrace(StackF.IsLastFrameFromForeignExceptionStackTrace(i));
+                    sfTemp.SetMethodBase(StackF.GetMethodBase(i));
+                    sfTemp.SetOffset(StackF.GetOffset(i));
+                    sfTemp.SetILOffset(StackF.GetILOffset(i));
 
-                        if (fNeedFileInfo)
-                        {
-                            sfTemp.SetFileName(StackF.GetFilename(i));
-                            sfTemp.SetLineNumber(StackF.GetLineNumber(i));
-                            sfTemp.SetColumnNumber(StackF.GetColumnNumber(i));
-                        }
+                    sfTemp.SetIsLastFrameFromForeignExceptionStackTrace(StackF.IsLastFrameFromForeignExceptionStackTrace(i));
 
-                        frames[i] = sfTemp;
+                    if (fNeedFileInfo)
+                    {
+                        sfTemp.SetFileName(StackF.GetFilename(i));
+                        sfTemp.SetLineNumber(StackF.GetLineNumber(i));
+                        sfTemp.SetColumnNumber(StackF.GetColumnNumber(i));
                     }
 
-                    // CalculateFramesToSkip skips all frames in the System.Diagnostics namespace,
-                    // but this is not desired if building a stack trace from an exception.
-                    if (e == null)
-                        m_iMethodsToSkip += CalculateFramesToSkip(StackF, m_iNumOfFrames);
+                    frames[i] = sfTemp;
+                }
 
-                    m_iNumOfFrames -= m_iMethodsToSkip;
-                    if (m_iNumOfFrames < 0)
-                    {
-                        m_iNumOfFrames = 0;
-                    }
+                // CalculateFramesToSkip skips all frames in the System.Diagnostics namespace,
+                // but this is not desired if building a stack trace from an exception.
+                if (e == null)
+                    m_iMethodsToSkip += CalculateFramesToSkip(StackF, m_iNumOfFrames);
+
+                m_iNumOfFrames -= m_iMethodsToSkip;
+                if (m_iNumOfFrames < 0)
+                {
+                    m_iNumOfFrames = 0;
                 }
+            }
 
-                // In case this is the same object being re-used, set frames to null
-                else
-                    frames = null;
+            // In case this is the same object being re-used, set frames to null
+            else
+            {
+                frames = null;
             }
         }
 
diff --git a/src/vm/debugdebugger.h b/src/vm/debugdebugger.h
index 14005e67f0..35758e185b 100644
--- a/src/vm/debugdebugger.h
+++ b/src/vm/debugdebugger.h
@@ -60,7 +60,6 @@ public:
 
     BOOLARRAYREF rgiLastFrameFromForeignExceptionStackTrace;
 
-    OBJECTREF getSourceLineInfo;
     int iFrameCount;
 
 protected:
diff --git a/src/vm/mscorlib.h b/src/vm/mscorlib.h
index 7642db6638..461c07df04 100644
--- a/src/vm/mscorlib.h
+++ b/src/vm/mscorlib.h
@@ -847,7 +847,6 @@ DEFINE_FIELD_U(rgFilename,                 StackFrameHelper,   rgFilename)
 DEFINE_FIELD_U(rgiLineNumber,              StackFrameHelper,   rgiLineNumber)
 DEFINE_FIELD_U(rgiColumnNumber,            StackFrameHelper,   rgiColumnNumber)
 DEFINE_FIELD_U(rgiLastFrameFromForeignExceptionStackTrace,            StackFrameHelper,   rgiLastFrameFromForeignExceptionStackTrace)
-DEFINE_FIELD_U(getSourceLineInfo,          StackFrameHelper,   getSourceLineInfo)
 DEFINE_FIELD_U(iFrameCount,                StackFrameHelper,   iFrameCount)
 
 DEFINE_CLASS(STREAM,                IO,                     Stream)