Allow provider deletion to be deferred until after tracing is stopped. (#11651)

diff --git a/src/scripts/genEventPipe.py b/src/scripts/genEventPipe.py
index 7fd1a45..a00511d 100644 (file)
--- a/src/scripts/genEventPipe.py
+++ b/src/scripts/genEventPipe.py
@@ -122,7 +122,7 @@ def generateClrEventPipeWriteEventsImpl(
     WriteEventImpl.append(
         "    EventPipeProvider" +
         providerPrettyName +
-        " = new EventPipeProvider(" +
+        " = EventPipe::CreateProvider(" +
         providerPrettyName +
         "GUID);\n")
     for eventNode in eventNodes:

diff --git a/src/vm/eventpipe.cpp b/src/vm/eventpipe.cpp
index 58f93ef..e7bef23 100644 (file)
--- a/src/vm/eventpipe.cpp
+++ b/src/vm/eventpipe.cpp
@@ -218,6 +218,10 @@ void EventPipe::Disable()
 
         // De-allocate buffers.
         s_pBufferManager->DeAllocateBuffers();
+
+        // Delete deferred providers.
+        // Providers can't be deleted during tracing because they may be needed when serializing the file.
+        s_pConfig->DeleteDeferredProviders();
     }
 }
 
@@ -234,6 +238,50 @@ bool EventPipe::Enabled()
     return enabled;
 }
 
+EventPipeProvider* EventPipe::CreateProvider(const GUID &providerID, EventPipeCallback pCallbackFunction, void *pCallbackData)
+{
+    CONTRACTL
+    {
+        THROWS;
+        GC_TRIGGERS;
+        MODE_ANY;
+    }
+    CONTRACTL_END;
+
+    return new EventPipeProvider(providerID, pCallbackFunction, pCallbackData);
+}
+
+void EventPipe::DeleteProvider(EventPipeProvider *pProvider)
+{
+    CONTRACTL
+    {
+        THROWS;
+        GC_TRIGGERS;
+        MODE_ANY;
+    }
+    CONTRACTL_END;
+
+    // Take the lock to make sure that we don't have a race
+    // between disabling tracing and deleting a provider
+    // where we hold a provider after tracing has been disabled.
+    CrstHolder _crst(GetLock());
+
+    if(pProvider != NULL)
+    {
+        if(Enabled())
+        {
+            // Save the provider until the end of the tracing session.
+            pProvider->SetDeleteDeferred();
+        }
+        else
+        {
+            // Delete the provider now.
+            // NOTE: This will remove it from all of the EventPipe data structures.
+            delete(pProvider);
+        }
+    }
+}
+
 void EventPipe::WriteEvent(EventPipeEvent &event, BYTE *pData, unsigned int length)
 {
     CONTRACTL
@@ -477,7 +525,7 @@ INT_PTR QCALLTYPE EventPipeInternal::CreateProvider(
 
     BEGIN_QCALL;
 
-    pProvider = new EventPipeProvider(providerID, pCallbackFunc, NULL);
+    pProvider = EventPipe::CreateProvider(providerID, pCallbackFunc, NULL);
 
     END_QCALL;
 
@@ -519,7 +567,7 @@ void QCALLTYPE EventPipeInternal::DeleteProvider(
     if(provHandle != NULL)
     {
         EventPipeProvider *pProvider = reinterpret_cast<EventPipeProvider*>(provHandle);
-        delete pProvider;
+        EventPipe::DeleteProvider(pProvider);
     }
 
     END_QCALL;

diff --git a/src/vm/eventpipe.h b/src/vm/eventpipe.h
index 0aced25..092508f 100644 (file)
--- a/src/vm/eventpipe.h
+++ b/src/vm/eventpipe.h
@@ -14,6 +14,7 @@ class EventPipeFile;
 class EventPipeJsonFile;
 class EventPipeBuffer;
 class EventPipeBufferManager;
+class EventPipeProvider;
 class MethodDesc;
 class SampleProfilerEventInstance;
 struct EventPipeProviderConfiguration;
@@ -179,6 +180,12 @@ class EventPipe
         // Specifies whether or not the event pipe is enabled.
         static bool Enabled();
 
+        // Create a provider.
+        static EventPipeProvider* CreateProvider(const GUID &providerID, EventPipeCallback pCallbackFunction = NULL, void *pCallbackData = NULL);
+
+        // Delete a provider.
+        static void DeleteProvider(EventPipeProvider *pProvider);
+
         // Write out an event.
         // Data is written as a serialized blob matching the ETW serialization conventions.
         static void WriteEvent(EventPipeEvent &event, BYTE *pData, unsigned int length);

diff --git a/src/vm/eventpipeconfiguration.cpp b/src/vm/eventpipeconfiguration.cpp
index a227d9d..73379fe 100644 (file)
--- 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 = new EventPipeProvider(s_configurationProviderID);
+    m_pConfigProvider = EventPipe::CreateProvider(s_configurationProviderID);
 
     // Create the metadata event.
     m_pMetadataEvent = m_pConfigProvider->AddEvent(
@@ -379,6 +379,33 @@ EventPipeEventInstance* EventPipeConfiguration::BuildEventMetadataEvent(EventPip
     return pInstance;
 }
 
+void EventPipeConfiguration::DeleteDeferredProviders()
+{
+    CONTRACTL
+    {
+        THROWS;
+        GC_TRIGGERS;
+        MODE_ANY;
+        // Lock must be held by EventPipe::Disable.
+        PRECONDITION(EventPipe::GetLock()->OwnedByCurrentThread());
+
+    }
+    CONTRACTL_END;
+
+    SListElem<EventPipeProvider*> *pElem = m_pProviderList->GetHead();
+    while(pElem != NULL)
+    {
+        EventPipeProvider *pProvider = pElem->GetValue();
+        if(pProvider->GetDeleteDeferred())
+        {
+            // The act of deleting the provider unregisters it and removes it from the list.
+            delete(pProvider);
+        }
+
+        pElem = m_pProviderList->GetNext(pElem);
+    }
+}
+
 EventPipeEnabledProviderList::EventPipeEnabledProviderList(
     EventPipeProviderConfiguration *pConfigs,
     unsigned int numConfigs)

diff --git a/src/vm/eventpipeconfiguration.h b/src/vm/eventpipeconfiguration.h
index 44c4205..de8e79d 100644 (file)
--- a/src/vm/eventpipeconfiguration.h
+++ b/src/vm/eventpipeconfiguration.h
@@ -71,6 +71,9 @@ public:
     // Get the event used to write metadata to the event stream.
     EventPipeEventInstance* BuildEventMetadataEvent(EventPipeEventInstance &sourceInstance);
 
+    // Delete deferred providers.
+    void DeleteDeferredProviders();
+
 private:
 
     // Get the provider without taking the lock.

diff --git a/src/vm/eventpipeprovider.cpp b/src/vm/eventpipeprovider.cpp
index 3c2d254..896f9b2 100644 (file)
--- a/src/vm/eventpipeprovider.cpp
+++ b/src/vm/eventpipeprovider.cpp
@@ -208,6 +208,18 @@ void EventPipeProvider::InvokeCallback()
     }
 }
 
+bool EventPipeProvider::GetDeleteDeferred() const
+{
+    LIMITED_METHOD_CONTRACT;
+    return m_deleteDeferred;
+}
+
+void EventPipeProvider::SetDeleteDeferred()
+{
+    LIMITED_METHOD_CONTRACT;
+    m_deleteDeferred = true;
+}
+
 void EventPipeProvider::RefreshAllEvents()
 {
     CONTRACTL

diff --git a/src/vm/eventpipeprovider.h b/src/vm/eventpipeprovider.h
index 605a82b..d2c459e 100644 (file)
--- a/src/vm/eventpipeprovider.h
+++ b/src/vm/eventpipeprovider.h
@@ -26,6 +26,7 @@ typedef void (*EventPipeCallback)(
 class EventPipeProvider
 {
     // Declare friends.
+    friend class EventPipe;
     friend class EventPipeConfiguration;
     friend class SampleProfiler;
 
@@ -55,9 +56,15 @@ private:
     // The configuration object.
     EventPipeConfiguration *m_pConfig;
 
-public:
+    // True if the provider has been deleted, but that deletion
+    // has been deferred until tracing is stopped.
+    bool m_deleteDeferred;
 
+    // Private constructor because all providers are created through EventPipe::CreateProvider.
     EventPipeProvider(const GUID &providerID, EventPipeCallback pCallbackFunction = NULL, void *pCallbackData = NULL);
+
+public:
+
     ~EventPipeProvider();
 
     // Get the provider ID.
@@ -96,6 +103,13 @@ public:
 
     // Invoke the provider callback.
     void InvokeCallback();
+
+    // Specifies whether or not the provider was deleted, but that deletion
+    // was deferred until after tracing is stopped.
+    bool GetDeleteDeferred() const;
+
+    // Defer deletion of the provider.
+    void SetDeleteDeferred();
 };
 
 #endif // FEATURE_PERFTRACING

diff --git a/src/vm/sampleprofiler.cpp b/src/vm/sampleprofiler.cpp
index c7373bc..bec757f 100644 (file)
--- a/src/vm/sampleprofiler.cpp
+++ b/src/vm/sampleprofiler.cpp
@@ -34,7 +34,7 @@ void SampleProfiler::Enable()
 
     if(s_pEventPipeProvider == NULL)
     {
-        s_pEventPipeProvider = new EventPipeProvider(s_providerID);
+        s_pEventPipeProvider = EventPipe::CreateProvider(s_providerID);
         s_pThreadTimeEvent = s_pEventPipeProvider->AddEvent(
             0, /* eventID */
             0, /* keywords */