Fix SIGSEGV in EventPipe on Shutdown (#14123)

backport cherrypick to fix https://github.sec.samsung.net/dotnet/home/issues/134

Change-Id: I0a2918d70c275fb81fea86a8b2eef613037666b5
Signed-off-by: chunseok lee <chunseok.lee@samsung.com>

diff --git a/packaging/0001-Fix-SIGSEGV-in-EventPipe-on-Shutdown-14123.patch b/packaging/0001-Fix-SIGSEGV-in-EventPipe-on-Shutdown-14123.patch
new file mode 100644 (file)
index 0000000..5473ae1
--- /dev/null
+++ b/packaging/0001-Fix-SIGSEGV-in-EventPipe-on-Shutdown-14123.patch
@@ -0,0 +1,201 @@
+From d9ad93dc3141a648d11f7d54bee59c318c97dd4e Mon Sep 17 00:00:00 2001
+From: Brian Robbins <brianrob@microsoft.com>
+Date: Fri, 22 Sep 2017 01:17:48 -0700
+Subject: [PATCH] Fix SIGSEGV in EventPipe on Shutdown (#14123)
+
+* Fix a crash that occurs when a provider is registered after the configuration object has been destroyed.
+
+* Code review feedback.
+---
+ src/vm/eventpipe.cpp              | 14 +++++++++---
+ src/vm/eventpipeconfiguration.cpp | 45 ++++++++++++++++++++++++++++++++++++++-
+ src/vm/eventpipeconfiguration.h   |  7 ++++++
+ src/vm/eventpipeprovider.cpp      | 18 +++-------------
+ src/vm/eventpipeprovider.h        |  2 +-
+ 5 files changed, 66 insertions(+), 20 deletions(-)
+
+diff --git a/src/vm/eventpipe.cpp b/src/vm/eventpipe.cpp
+index e041615..50909a1 100644
+--- a/src/vm/eventpipe.cpp
++++ b/src/vm/eventpipe.cpp
+@@ -248,7 +248,13 @@ EventPipeProvider* EventPipe::CreateProvider(const GUID &providerID, EventPipeCa
+     }
+     CONTRACTL_END;
+ 
+-    return new EventPipeProvider(providerID, pCallbackFunction, pCallbackData);
++    EventPipeProvider *pProvider = NULL;
++    if (s_pConfig != NULL)
++    {
++        pProvider = s_pConfig->CreateProvider(providerID, pCallbackFunction, pCallbackData);
++    }
++
++    return pProvider;
+ }
+ 
+ void EventPipe::DeleteProvider(EventPipeProvider *pProvider)
+@@ -276,8 +282,10 @@ void EventPipe::DeleteProvider(EventPipeProvider *pProvider)
+         else
+         {
+             // Delete the provider now.
+-            // NOTE: This will remove it from all of the EventPipe data structures.
+-            delete(pProvider);
++            if (s_pConfig != NULL)
++            {
++                s_pConfig->DeleteProvider(pProvider);
++            }
+         }
+     }
+ }
+diff --git a/src/vm/eventpipeconfiguration.cpp b/src/vm/eventpipeconfiguration.cpp
+index 42f9daf..69e65e6 100644
+--- a/src/vm/eventpipeconfiguration.cpp
++++ b/src/vm/eventpipeconfiguration.cpp
+@@ -59,7 +59,7 @@ void EventPipeConfiguration::Initialize()
+     CONTRACTL_END;
+ 
+     // Create the configuration provider.
+-    m_pConfigProvider = EventPipe::CreateProvider(s_configurationProviderID);
++    m_pConfigProvider = CreateProvider(s_configurationProviderID, NULL, NULL);
+ 
+     // Create the metadata event.
+     m_pMetadataEvent = m_pConfigProvider->AddEvent(
+@@ -70,6 +70,49 @@ void EventPipeConfiguration::Initialize()
+         false); /* needStack */
+ }
+ 
++EventPipeProvider* EventPipeConfiguration::CreateProvider(const GUID &providerID, EventPipeCallback pCallbackFunction, void *pCallbackData)
++{
++    CONTRACTL
++    {
++        THROWS;
++        GC_NOTRIGGER;
++        MODE_ANY;
++    }
++    CONTRACTL_END;
++
++    // Allocate a new provider.
++    EventPipeProvider *pProvider = new EventPipeProvider(this, providerID, pCallbackFunction, pCallbackData);
++
++    // Register the provider with the configuration system.
++    RegisterProvider(*pProvider);
++
++    return pProvider;
++}
++
++void EventPipeConfiguration::DeleteProvider(EventPipeProvider *pProvider)
++{
++    CONTRACTL
++    {
++        THROWS;
++        GC_NOTRIGGER;
++        MODE_ANY;
++        PRECONDITION(pProvider != NULL);
++    }
++    CONTRACTL_END;
++
++    if (pProvider == NULL)
++    {
++        return;
++    }
++
++    // Unregister the provider.
++    UnregisterProvider(*pProvider);
++
++    // Free the provider itself.
++    delete(pProvider);
++}
++
++
+ bool EventPipeConfiguration::RegisterProvider(EventPipeProvider &provider)
+ {
+     CONTRACTL
+diff --git a/src/vm/eventpipeconfiguration.h b/src/vm/eventpipeconfiguration.h
+index de8e79d..96be50e 100644
+--- a/src/vm/eventpipeconfiguration.h
++++ b/src/vm/eventpipeconfiguration.h
+@@ -6,6 +6,7 @@
+ 
+ #ifdef FEATURE_PERFTRACING
+ 
++#include "eventpipe.h"
+ #include "slist.h"
+ 
+ class EventPipeEnabledProvider;
+@@ -35,6 +36,12 @@ public:
+     // Perform initialization that cannot be performed in the constructor.
+     void Initialize();
+ 
++    // Create a new provider.
++    EventPipeProvider* CreateProvider(const GUID &providerID, EventPipeCallback pCallbackFunction, void *pCallbackData);
++
++    // Delete a provider.
++    void DeleteProvider(EventPipeProvider *pProvider);
++
+     // Register a provider.
+     bool RegisterProvider(EventPipeProvider &provider);
+ 
+diff --git a/src/vm/eventpipeprovider.cpp b/src/vm/eventpipeprovider.cpp
+index 896f9b26..4cc02c1 100644
+--- a/src/vm/eventpipeprovider.cpp
++++ b/src/vm/eventpipeprovider.cpp
+@@ -10,13 +10,14 @@
+ 
+ #ifdef FEATURE_PERFTRACING
+ 
+-EventPipeProvider::EventPipeProvider(const GUID &providerID, EventPipeCallback pCallbackFunction, void *pCallbackData)
++EventPipeProvider::EventPipeProvider(EventPipeConfiguration *pConfig, const GUID &providerID, EventPipeCallback pCallbackFunction, void *pCallbackData)
+ {
+     CONTRACTL
+     {
+         THROWS;
+         GC_NOTRIGGER;
+         MODE_ANY;
++        PRECONDITION(pConfig != NULL);
+     }
+     CONTRACTL_END;
+ 
+@@ -27,11 +28,7 @@ EventPipeProvider::EventPipeProvider(const GUID &providerID, EventPipeCallback p
+     m_pEventList = new SList<SListElem<EventPipeEvent*>>();
+     m_pCallbackFunction = pCallbackFunction;
+     m_pCallbackData = pCallbackData;
+-    m_pConfig = EventPipe::GetConfiguration();
+-    _ASSERTE(m_pConfig != NULL);
+-
+-    // Register the provider.
+-    m_pConfig->RegisterProvider(*this);
++    m_pConfig = pConfig;
+ }
+ 
+ EventPipeProvider::~EventPipeProvider()
+@@ -44,15 +41,6 @@ EventPipeProvider::~EventPipeProvider()
+     }
+     CONTRACTL_END;
+ 
+-    // Unregister the provider.
+-    // This call is re-entrant.
+-    // NOTE: We don't use the cached event pipe configuration pointer
+-    // in case this runs during shutdown and the configuration has already
+-    // been freed.
+-    EventPipeConfiguration* pConfig = EventPipe::GetConfiguration();
+-    _ASSERTE(pConfig != NULL);
+-    pConfig->UnregisterProvider(*this);
+-
+     // Free all of the events.
+     if(m_pEventList != NULL)
+     {
+diff --git a/src/vm/eventpipeprovider.h b/src/vm/eventpipeprovider.h
+index d2c459e..b0e9cc9 100644
+--- a/src/vm/eventpipeprovider.h
++++ b/src/vm/eventpipeprovider.h
+@@ -61,7 +61,7 @@ private:
+     bool m_deleteDeferred;
+ 
+     // Private constructor because all providers are created through EventPipe::CreateProvider.
+-    EventPipeProvider(const GUID &providerID, EventPipeCallback pCallbackFunction = NULL, void *pCallbackData = NULL);
++    EventPipeProvider(EventPipeConfiguration *pConfig, const GUID &providerID, EventPipeCallback pCallbackFunction = NULL, void *pCallbackData = NULL);
+ 
+ public:
+ 
+-- 
+2.7.4
+

diff --git a/packaging/coreclr.spec b/packaging/coreclr.spec
index 6acedf5..dbb6c65 100644 (file)
--- a/packaging/coreclr.spec
+++ b/packaging/coreclr.spec
@@ -23,7 +23,7 @@ Source1000: downloaded_files.tar.gz
 Source1001: %{name}.manifest
 Source1002: libicu.tar.gz
 Source1003: dep_libs.tar.gz
-# Gbp-Ignore-Patches: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40
+# Gbp-Ignore-Patches: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41
 Patch0:     0001-Add-project.assets.json-files.patch
 Patch1:     0001-ARM-Linux-Support-unaligned-struct-read-write-11290.patch
 Patch2:     0002-x86-Linux-Thread-safe-UMThunkMarshInfo-RunTimeInit-1.patch
@@ -65,6 +65,7 @@ Patch37:     0001-Use-addresses-without-sign-extension-in-lldb-plugin-.patch
 Patch38:     0001-Fix-uaf-in-DestroyThread-function.patch
 Patch39:     0001-Enable-gdbjit-while-NI-file-exist.patch
 Patch40:     0001-Fix-crossgen-debug-directory-generation-problems.-12.patch
+Patch41:     0001-Fix-SIGSEGV-in-EventPipe-on-Shutdown-14123.patch
 
 ExcludeArch: aarch64
 
@@ -204,6 +205,7 @@ cp %{SOURCE1001} .
 %patch38 -p1
 %patch39 -p1
 %patch40 -p1
+%patch41 -p1
 
 %if 0%{skipmscorlib}
 %else