Fix memory leak from managed exceptions

Handling thrown PAL_SEHException was causing leaks for all exceptions thrown due to
two aspects:
1) PAL_GetStackBase() and PAL_GetStackLimit() were missing calls to pthread_attr_destroy()
2) We were calling the DispatchManagedException from C++ catch handlers and this function
   never returns. So the C++ exception handling never called __cxa_end_catch that is
   responsible for freeing the exception storage allocated by the C++ runtime.
The fix to the 2nd aspect was to store a copy of the exception in the catch handler, let it
complete and then call the DispatchManagedException with the copy. It was also necessary to
slightly modify the unwinding of sequences of native frames since there is now no rethrowable
exception and the StartUnwindingManagedFrames has to throw a new one.
This change has a secondary benefit - the StartUnwindingManagedFrames no longer calls
__cxa_rethrow, but rather a helper C++ function that uses regular "throw" keyword.
That makes the code more portable.

5 files changed, 68 insertions, 24 deletions

diff --git a/src/pal/inc/pal.h b/src/pal/inc/pal.h
index 5d8e56e0ef..43fc0694f5 100644
--- a/src/pal/inc/pal.h
+++ b/src/pal/inc/pal.h
@@ -6364,6 +6364,25 @@ public:
         ExceptionRecord = *pExceptionRecord;
         ContextRecord = *pContextRecord;
     }
+
+    PAL_SEHException()
+    {
+    }    
+
+    PAL_SEHException(const PAL_SEHException& ex)
+    {
+        *this = ex;
+    }    
+
+    PAL_SEHException& operator=(const PAL_SEHException& ex)
+    {
+        ExceptionPointers.ExceptionRecord = &ExceptionRecord;
+        ExceptionPointers.ContextRecord = &ContextRecord;
+        ExceptionRecord = ex.ExceptionRecord;
+        ContextRecord = ex.ContextRecord;
+
+        return *this;
+    }    
 };
 
 typedef VOID (PALAPI *PHARDWARE_EXCEPTION_HANDLER)(PAL_SEHException* ex);
diff --git a/src/pal/src/thread/thread.cpp b/src/pal/src/thread/thread.cpp
index 2c7b2388a8..33dbfbc914 100644
--- a/src/pal/src/thread/thread.cpp
+++ b/src/pal/src/thread/thread.cpp
@@ -2446,6 +2446,9 @@ PAL_GetStackBase()
     status = pthread_attr_getstack(&attr, &stackAddr, &stackSize);
     _ASSERT_MSG(status == 0, "pthread_attr_getstack call failed");
 
+    status = pthread_attr_destroy(&attr);
+    _ASSERT_MSG(status == 0, "pthread_attr_destroy call failed");
+
     return (void*)((size_t)stackAddr + stackSize);
 #endif
 }
@@ -2480,6 +2483,9 @@ PAL_GetStackLimit()
 
     status = pthread_attr_getstack(&attr, &stackAddr, &stackSize);
     _ASSERT_MSG(status == 0, "pthread_attr_getstack call failed");
+
+    status = pthread_attr_destroy(&attr);
+    _ASSERT_MSG(status == 0, "pthread_attr_destroy call failed");
     
     return stackAddr;
 #endif
diff --git a/src/vm/amd64/unixasmhelpers.S b/src/vm/amd64/unixasmhelpers.S
index e3f4adc259..ccf552367c 100644
--- a/src/vm/amd64/unixasmhelpers.S
+++ b/src/vm/amd64/unixasmhelpers.S
@@ -291,7 +291,7 @@ LEAF_END SinglecastDelegateInvokeStub, _TEXT
 // from the passed in context and finally sets the RSP to that frame and sets the return
 // address to the target frame's RIP.
 //
-// EXTERN_C void StartUnwindingNativeFrames(CONTEXT* context);
+// EXTERN_C void StartUnwindingNativeFrames(CONTEXT* context, PAL_SEHException* ex);
 LEAF_ENTRY StartUnwindingNativeFrames, _TEXT
         // Save the RBP to the stack so that the unwind can work at the instruction after
         // loading the RBP from the context, but before loading the RSP from the context.
@@ -316,6 +316,8 @@ LEAF_ENTRY StartUnwindingNativeFrames, _TEXT
         // Store return address to the stack
         push_register rax
         push_nonvol_reg rbp
-        call    EXTERNAL_C_FUNC(__cxa_rethrow)
+        // The PAL_SEHException pointer
+        mov     rdi, rsi 
+        call    EXTERNAL_C_FUNC(ThrowExceptionHelper)
 LEAF_END StartUnwindingNativeFrames, _TEXT
 
diff --git a/src/vm/exceptionhandling.cpp b/src/vm/exceptionhandling.cpp
index 5d5f3e61c0..982c3e06d5 100644
--- a/src/vm/exceptionhandling.cpp
+++ b/src/vm/exceptionhandling.cpp
@@ -3631,7 +3631,7 @@ ExceptionTracker* ExceptionTracker::GetOrCreateTracker(
             fIsRethrow = true;
         }
         else
-        if (pTracker->m_ptrs.ExceptionRecord != pExceptionRecord)
+        if ((pTracker->m_ptrs.ExceptionRecord != pExceptionRecord) && !fIsInterleavedHandling)
         {
             EH_LOG((LL_INFO100, ">>NEW exception (exception records do not match)\n"));
             fCreateNewTracker = true;
@@ -4396,7 +4396,8 @@ bool IsSpInStackLimits(ULONG64 sp, ULONG64 stackLowAddress, ULONG64 stackHighAdd
 //
 // Arguments:
 //      context - context at which to start the native unwinding
-extern "C" void StartUnwindingNativeFrames(CONTEXT* context);
+//      ex      - pointer to the exception to use to unwind the native frames
+extern "C" void StartUnwindingNativeFrames(CONTEXT* context, PAL_SEHException* ex);
 
 //---------------------------------------------------------------------------------------
 //
@@ -4678,7 +4679,7 @@ VOID DECLSPEC_NORETURN UnwindManagedExceptionPass1(PAL_SEHException& ex)
 
             // Now we need to unwind the native frames until we reach managed frames again or the exception is
             // handled in the native code.
-            StartUnwindingNativeFrames(&frameContext);
+            StartUnwindingNativeFrames(&frameContext, &ex);
             UNREACHABLE();
         }
 
@@ -4688,16 +4689,35 @@ VOID DECLSPEC_NORETURN UnwindManagedExceptionPass1(PAL_SEHException& ex)
     EEPOLICY_HANDLE_FATAL_ERROR(COR_E_EXECUTIONENGINE);
 }
 
+//---------------------------------------------------------------------------------------
+//
+// Helper function to throw the passed in exception.
+// It is called from the assembler function StartUnwindingNativeFrames 
+// Arguments:
+//
+//      ex - the exception to throw.
+//
+extern "C"
+void ThrowExceptionHelper(PAL_SEHException* ex)
+{
+    throw *ex;
+}
+
 VOID DECLSPEC_NORETURN DispatchManagedException(PAL_SEHException& ex)
 {
-    try
-    {
-        UnwindManagedExceptionPass1(ex);
-    }
-    catch (PAL_SEHException& ex)
+    do
     {
-        DispatchManagedException(ex);
+        try
+        {
+            UnwindManagedExceptionPass1(ex);
+            UNREACHABLE();
+        }
+        catch (PAL_SEHException& ex2)
+        {
+            ex = ex2;
+        }
     }
+    while (true);
 }
 
 #ifdef _AMD64_
@@ -5064,17 +5084,7 @@ VOID PALAPI HandleHardwareException(PAL_SEHException* ex)
             }
 #endif //_AMD64_
 
-            // We throw the exception and catch it right away so that in case the DispatchManagedException
-            // needs to cross managed to native stack frames boundary, there is an exception that can
-            // be rethrow in the StartUnwindingNativeFrames.
-            try
-            {
-                throw *ex;
-            }
-            catch (PAL_SEHException& ex2)
-            {
-                DispatchManagedException(ex2);
-            }
+            DispatchManagedException(*ex);
             UNREACHABLE();
         }
     }
diff --git a/src/vm/exceptmacros.h b/src/vm/exceptmacros.h
index 0d162951eb..6589205651 100644
--- a/src/vm/exceptmacros.h
+++ b/src/vm/exceptmacros.h
@@ -323,14 +323,21 @@ VOID DECLSPEC_NORETURN UnwindAndContinueRethrowHelperAfterCatch(Frame* pEntryFra
 VOID DECLSPEC_NORETURN DispatchManagedException(PAL_SEHException& ex);
 
 #define INSTALL_MANAGED_EXCEPTION_DISPATCHER        \
-        try { \
+        PAL_SEHException exCopy;                    \
+        bool hasCaughtException = false;            \
+        try {
 
 #define UNINSTALL_MANAGED_EXCEPTION_DISPATCHER      \
         }                                           \
         catch (PAL_SEHException& ex)                \
         {                                           \
-            DispatchManagedException(ex);           \
+            exCopy = ex;                            \
+            hasCaughtException = true;              \
         }                                           \
+        if (hasCaughtException)                     \
+        {                                           \
+            DispatchManagedException(exCopy);       \
+        }
 
 #else