Revert "Fix SIGSEGV in EventPipe on Shutdown (#14123) (#15765)" (#16084)

This reverts commit 381e6b1.

Author: Victor "Nate" Graf

src/vm/eventpipe.cpp

@@ -248,13 +248,7 @@ EventPipeProvider* EventPipe::CreateProvider(const GUID &providerID, EventPipeCa
     }
     CONTRACTL_END;
 
-    EventPipeProvider *pProvider = NULL;
-    if (s_pConfig != NULL)
-    {
-        pProvider = s_pConfig->CreateProvider(providerID, pCallbackFunction, pCallbackData);
-    }
-
-    return pProvider;
+    return new EventPipeProvider(providerID, pCallbackFunction, pCallbackData);
 }
 
 void EventPipe::DeleteProvider(EventPipeProvider *pProvider)
@@ -282,10 +276,8 @@ void EventPipe::DeleteProvider(EventPipeProvider *pProvider)
         else
         {
             // Delete the provider now.
-            if (s_pConfig != NULL)
-            {
-                s_pConfig->DeleteProvider(pProvider);
-            }
+            // NOTE: This will remove it from all of the EventPipe data structures.
+            delete(pProvider);
         }
     }
 }

src/vm/eventpipeconfiguration.cpp

@@ -34,11 +34,6 @@ EventPipeConfiguration::~EventPipeConfiguration()
         MODE_ANY;
     }
     CONTRACTL_END;
-    
-    if(m_pConfigProvider != NULL)
-    {
-        DeleteProvider(m_pConfigProvider);
-    }
 
     if(m_pEnabledProviderList != NULL)
     {
@@ -64,7 +59,7 @@ void EventPipeConfiguration::Initialize()
     CONTRACTL_END;
 
     // Create the configuration provider.
-    m_pConfigProvider = CreateProvider(s_configurationProviderID, NULL, NULL);
+    m_pConfigProvider = EventPipe::CreateProvider(s_configurationProviderID);
 
     // Create the metadata event.
     m_pMetadataEvent = m_pConfigProvider->AddEvent(
@@ -75,49 +70,6 @@ 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
@@ -448,7 +400,8 @@ void EventPipeConfiguration::DeleteDeferredProviders()
         EventPipeProvider *pProvider = pElem->GetValue();
         if(pProvider->GetDeleteDeferred())
         {
-            DeleteProvider(pProvider);
+            // The act of deleting the provider unregisters it and removes it from the list.
+            delete(pProvider);
         }
 
         pElem = m_pProviderList->GetNext(pElem);

src/vm/eventpipeconfiguration.h

@@ -6,7 +6,6 @@
 
 #ifdef FEATURE_PERFTRACING
 
-#include "eventpipe.h"
 #include "slist.h"
 
 class EventPipeEnabledProvider;
@@ -36,12 +35,6 @@ class EventPipeConfiguration
     // 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);
 

src/vm/eventpipeprovider.cpp

@@ -10,14 +10,13 @@
 
 #ifdef FEATURE_PERFTRACING
 
-EventPipeProvider::EventPipeProvider(EventPipeConfiguration *pConfig, const GUID &providerID, EventPipeCallback pCallbackFunction, void *pCallbackData)
+EventPipeProvider::EventPipeProvider(const GUID &providerID, EventPipeCallback pCallbackFunction, void *pCallbackData)
 {
     CONTRACTL
     {
         THROWS;
         GC_NOTRIGGER;
         MODE_ANY;
-        PRECONDITION(pConfig != NULL);
     }
     CONTRACTL_END;
 
@@ -28,7 +27,11 @@ EventPipeProvider::EventPipeProvider(EventPipeConfiguration *pConfig, const GUID
     m_pEventList = new SList<SListElem<EventPipeEvent*>>();
     m_pCallbackFunction = pCallbackFunction;
     m_pCallbackData = pCallbackData;
-    m_pConfig = pConfig;
+    m_pConfig = EventPipe::GetConfiguration();
+    _ASSERTE(m_pConfig != NULL);
+
+    // Register the provider.
+    m_pConfig->RegisterProvider(*this);
 }
 
 EventPipeProvider::~EventPipeProvider()
@@ -41,6 +44,15 @@ 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)
     {

src/vm/eventpipeprovider.h

@@ -61,7 +61,7 @@ class EventPipeProvider
     bool m_deleteDeferred;
 
     // Private constructor because all providers are created through EventPipe::CreateProvider.
-    EventPipeProvider(EventPipeConfiguration *pConfig, const GUID &providerID, EventPipeCallback pCallbackFunction = NULL, void *pCallbackData = NULL);
+    EventPipeProvider(const GUID &providerID, EventPipeCallback pCallbackFunction = NULL, void *pCallbackData = NULL);
 
 public: