Refactored the duplicated code into a template method pattern

4 files changed, 116 insertions, 130 deletions

diff --git a/src/vm/eventpipe.cpp b/src/vm/eventpipe.cpp
index c8f2655a41..361fe837c3 100644
--- a/src/vm/eventpipe.cpp
+++ b/src/vm/eventpipe.cpp
@@ -246,28 +246,21 @@ EventPipeSessionID EventPipe::Enable(
     CONTRACTL_END;
 
     EventPipeSessionID sessionId;
-    EventPipeProviderCallbackDataQueue eventPipeProviderCallbackDataQueue;
-    EventPipeProviderCallbackData eventPipeProviderCallbackData;
-    {
-        // Take the lock before enabling tracing.
-        CrstHolder _crst(GetLock());
-
-        // Create a new session.
-        SampleProfiler::SetSamplingRate((unsigned long)profilerSamplingRateInNanoseconds);
-        EventPipeSession *pSession = s_pConfig->CreateSession(
-            (strOutputPath != NULL) ? EventPipeSessionType::File : EventPipeSessionType::Streaming,
-            circularBufferSizeInMB,
-            pProviders,
-            numProviders);
-
-        // Enable the session.
-        sessionId = Enable(strOutputPath, pSession, sessionType, pStream, &eventPipeProviderCallbackDataQueue);
-    }
-
-    while (eventPipeProviderCallbackDataQueue.TryDequeue(&eventPipeProviderCallbackData))
-    {
-        EventPipeProvider::InvokeCallback(eventPipeProviderCallbackData);
-    }
+    EventPipe::RunWithCallbackPostponed(
+        [&](EventPipeProviderCallbackDataQueue* pEventPipeProviderCallbackDataQueue)
+        {
+            // Create a new session.
+            SampleProfiler::SetSamplingRate((unsigned long)profilerSamplingRateInNanoseconds);
+            EventPipeSession *pSession = s_pConfig->CreateSession(
+                (strOutputPath != NULL) ? EventPipeSessionType::File : EventPipeSessionType::Streaming,
+                circularBufferSizeInMB,
+                pProviders,
+                numProviders);
+
+            // Enable the session.
+            sessionId = Enable(strOutputPath, pSession, sessionType, pStream, pEventPipeProviderCallbackDataQueue);
+        }
+    );
 
     return sessionId;
 }
@@ -356,18 +349,12 @@ void EventPipe::Disable(EventPipeSessionID id)
     // Don't block GC during clean-up.
     GCX_PREEMP();
 
-    EventPipeProviderCallbackDataQueue eventPipeProviderCallbackDataQueue;
-    EventPipeProviderCallbackData eventPipeProviderCallbackData;
-    {
-        // Take the lock before disabling tracing.
-        CrstHolder _crst(GetLock());
-        DisableInternal(reinterpret_cast<EventPipeSessionID>(s_pSession), &eventPipeProviderCallbackDataQueue);
-    }
-
-    while (eventPipeProviderCallbackDataQueue.TryDequeue(&eventPipeProviderCallbackData))
-    {
-        EventPipeProvider::InvokeCallback(eventPipeProviderCallbackData);
-    }
+    EventPipe::RunWithCallbackPostponed(
+        [&](EventPipeProviderCallbackDataQueue* pEventPipeProviderCallbackDataQueue)
+        {
+            DisableInternal(reinterpret_cast<EventPipeSessionID>(s_pSession), pEventPipeProviderCallbackDataQueue);
+        }
+    );
 }
 
 void EventPipe::DisableInternal(EventPipeSessionID id, EventPipeProviderCallbackDataQueue* pEventPipeProviderCallbackDataQueue)
@@ -534,46 +521,39 @@ void WINAPI EventPipe::FlushTimer(PVOID parameter, BOOLEAN timerFired)
 
     GCX_PREEMP();
 
-    EventPipeProviderCallbackDataQueue eventPipeProviderCallbackDataQueue;
-    EventPipeProviderCallbackData eventPipeProviderCallbackData;
-    {
-        // Take the lock control lock to make sure that tracing isn't disabled during this operation.
-        CrstHolder _crst(GetLock());
+    EventPipe::RunWithCallbackPostponed(
+        [&](EventPipeProviderCallbackDataQueue* pEventPipeProviderCallbackDataQueue)
+        {
+            if (s_pSession == nullptr || s_pFile == nullptr)
+                return;
 
-        if (s_pSession == nullptr || s_pFile == nullptr)
-            return;
+            // Make sure that we should actually switch files.
+            if (!Enabled() || s_pSession->GetSessionType() != EventPipeSessionType::IpcStream)
+                return;
 
-        // Make sure that we should actually switch files.
-        if (!Enabled() || s_pSession->GetSessionType() != EventPipeSessionType::IpcStream)
-            return;
-
-        if (CLRGetTickCount64() > (s_lastFlushSwitchTime + 100))
-        {
-            // Get the current time stamp.
-            // WriteAllBuffersToFile will use this to ensure that no events after
-            // the current timestamp are written into the file.
-            LARGE_INTEGER stopTimeStamp;
-            QueryPerformanceCounter(&stopTimeStamp);
-            s_pBufferManager->WriteAllBuffersToFile(s_pFile, stopTimeStamp);
-
-            s_lastFlushSwitchTime = CLRGetTickCount64();
-        }
+            if (CLRGetTickCount64() > (s_lastFlushSwitchTime + 100))
+            {
+                // Get the current time stamp.
+                // WriteAllBuffersToFile will use this to ensure that no events after
+                // the current timestamp are written into the file.
+                LARGE_INTEGER stopTimeStamp;
+                QueryPerformanceCounter(&stopTimeStamp);
+                s_pBufferManager->WriteAllBuffersToFile(s_pFile, stopTimeStamp);
+
+                s_lastFlushSwitchTime = CLRGetTickCount64();
+            }
 
-        if (s_pFile->HasErrors())
-        {
-            EX_TRY
+            if (s_pFile->HasErrors())
             {
-                DisableInternal(reinterpret_cast<EventPipeSessionID>(s_pSession), &eventPipeProviderCallbackDataQueue);
+                EX_TRY
+                {
+                    DisableInternal(reinterpret_cast<EventPipeSessionID>(s_pSession), pEventPipeProviderCallbackDataQueue);
+                }
+                EX_CATCH {}
+                EX_END_CATCH(SwallowAllExceptions);
             }
-            EX_CATCH {}
-            EX_END_CATCH(SwallowAllExceptions);
         }
-    }
-
-    while (eventPipeProviderCallbackDataQueue.TryDequeue(&eventPipeProviderCallbackData))
-    {
-        EventPipeProvider::InvokeCallback(eventPipeProviderCallbackData);
-    }
+    );
 }
 
 EventPipeSession *EventPipe::GetSession(EventPipeSessionID id)
@@ -613,16 +593,12 @@ EventPipeProvider *EventPipe::CreateProvider(const SString &providerName, EventP
     CONTRACTL_END;
     
     EventPipeProvider *pProvider = NULL;
-    EventPipeProviderCallbackDataQueue eventPipeProviderCallbackDataQueue;
-    EventPipeProviderCallbackData eventPipeProviderCallbackData;
-    {
-        CrstHolder _crst(GetLock());
-        pProvider = EventPipe::CreateProvider(providerName, pCallbackFunction, pCallbackData, &eventPipeProviderCallbackDataQueue);
-    }
-    while (eventPipeProviderCallbackDataQueue.TryDequeue(&eventPipeProviderCallbackData))
-    {
-        EventPipeProvider::InvokeCallback(eventPipeProviderCallbackData);
-    }
+    EventPipe::RunWithCallbackPostponed(
+        [&](EventPipeProviderCallbackDataQueue* pEventPipeProviderCallbackDataQueue)
+        {
+            pProvider = EventPipe::CreateProvider(providerName, pCallbackFunction, pCallbackData, pEventPipeProviderCallbackDataQueue);
+        }
+    );
 
     return pProvider;
 }
@@ -930,4 +906,9 @@ EventPipeEventInstance *EventPipe::GetNextEvent()
     return pInstance;
 }
 
+/* static */ void EventPipe::InvokeCallback(EventPipeProviderCallbackData eventPipeProviderCallbackData)
+{
+    EventPipeProvider::InvokeCallback(eventPipeProviderCallbackData);
+}
+
 #endif // FEATURE_PERFTRACING
diff --git a/src/vm/eventpipe.h b/src/vm/eventpipe.h
index 6ddcc59754..ad0e84e2ad 100644
--- a/src/vm/eventpipe.h
+++ b/src/vm/eventpipe.h
@@ -23,6 +23,16 @@ class EventPipeSession;
 class IpcStream;
 enum class EventPipeSessionType;
 
+enum class EventPipeEventLevel
+{
+    LogAlways,
+    Critical,
+    Error,
+    Warning,
+    Informational,
+    Verbose
+};
+
 // EVENT_FILTER_DESCRIPTOR (This type does not exist on non-Windows platforms.)
 //  https://docs.microsoft.com/en-us/windows/desktop/api/evntprov/ns-evntprov-_event_filter_descriptor
 //  The structure supplements the event provider, level, and keyword data that
@@ -234,7 +244,28 @@ public:
 
 typedef UINT64 EventPipeSessionID;
 
-class EventPipeProviderCallbackDataQueue;
+struct EventPipeProviderCallbackData
+{
+    LPCWSTR pFilterData;
+    EventPipeCallback pCallbackFunction;
+    bool enabled;
+    INT64 keywords;
+    EventPipeEventLevel providerLevel;
+    void* pCallbackData;
+};
+
+class EventPipeProviderCallbackDataQueue
+{
+public:
+    EventPipeProviderCallbackDataQueue();
+
+    void Enqueue(EventPipeProviderCallbackData* pEventPipeProviderCallbackData);
+
+    bool TryDequeue(EventPipeProviderCallbackData* pEventPipeProviderCallbackData);
+
+private:
+    SList<SListElem<EventPipeProviderCallbackData>> list;
+};
 
 class EventPipe
 {
@@ -302,6 +333,24 @@ public:
     // Get next event.
     static EventPipeEventInstance *GetNextEvent();
 
+    template<class T>
+    static void RunWithCallbackPostponed(T f)
+    {
+        EventPipeProviderCallbackDataQueue eventPipeProviderCallbackDataQueue;
+        EventPipeProviderCallbackData eventPipeProviderCallbackData;
+        {
+            CrstHolder _crst(GetLock());
+            f(&eventPipeProviderCallbackDataQueue);
+        }
+
+        while (eventPipeProviderCallbackDataQueue.TryDequeue(&eventPipeProviderCallbackData))
+        {
+            EventPipe::InvokeCallback(eventPipeProviderCallbackData);
+        }
+    }
+
+    static void InvokeCallback(EventPipeProviderCallbackData eventPipeProviderCallbackData);
+
 private:
     // The counterpart to WriteEvent which after the payload is constructed
     static void WriteEventInternal(EventPipeEvent &event, EventPipeEventPayload &payload, LPCGUID pActivityId = NULL, LPCGUID pRelatedActivityId = NULL);
diff --git a/src/vm/eventpipeconfiguration.cpp b/src/vm/eventpipeconfiguration.cpp
index 9fb09b9ed6..b32b94a5f8 100644
--- a/src/vm/eventpipeconfiguration.cpp
+++ b/src/vm/eventpipeconfiguration.cpp
@@ -92,18 +92,13 @@ void EventPipeConfiguration::Initialize()
     }
     CONTRACTL_END;
     
-    EventPipeProviderCallbackDataQueue eventPipeProviderCallbackDataQueue;
-    EventPipeProviderCallbackData eventPipeProviderCallbackData;
-    {
-        CrstHolder _crst(EventPipe::GetLock());
-
-        // Create the configuration provider.
-        m_pConfigProvider = CreateProvider(SL(s_configurationProviderName), NULL, NULL, &eventPipeProviderCallbackDataQueue);
-    }
-    while (eventPipeProviderCallbackDataQueue.TryDequeue(&eventPipeProviderCallbackData))
-    {
-        EventPipeProvider::InvokeCallback(eventPipeProviderCallbackData);
-    }
+    EventPipe::RunWithCallbackPostponed(
+        [&](EventPipeProviderCallbackDataQueue* pEventPipeProviderCallbackDataQueue)
+        {
+            // Create the configuration provider.
+            m_pConfigProvider = CreateProvider(SL(s_configurationProviderName), NULL, NULL, pEventPipeProviderCallbackDataQueue);
+        }
+    );
 
     // Create the metadata event.
     m_pMetadataEvent = m_pConfigProvider->AddEvent(
diff --git a/src/vm/eventpipeconfiguration.h b/src/vm/eventpipeconfiguration.h
index e1e71e9524..eb694cec4d 100644
--- a/src/vm/eventpipeconfiguration.h
+++ b/src/vm/eventpipeconfiguration.h
@@ -16,45 +16,6 @@ class EventPipeProvider;
 class EventPipeSession;
 enum class EventPipeSessionType;
 
-enum class EventPipeEventLevel
-{
-    LogAlways,
-    Critical,
-    Error,
-    Warning,
-    Informational,
-    Verbose
-};
-
-struct EventPipeProviderCallbackData
-{
-    LPCWSTR pFilterData;
-    EventPipeCallback pCallbackFunction;
-    bool enabled;
-    INT64 keywords;
-    EventPipeEventLevel providerLevel;
-    void* pCallbackData;
-};
-
-struct EventPipeProviderCallbackDataNode
-{
-    EventPipeProviderCallbackData* value;
-    EventPipeProviderCallbackDataNode* prev;
-};
-
-class EventPipeProviderCallbackDataQueue
-{
-public:
-    EventPipeProviderCallbackDataQueue();
-
-    void Enqueue(EventPipeProviderCallbackData* pEventPipeProviderCallbackData);
-
-    bool TryDequeue(EventPipeProviderCallbackData* pEventPipeProviderCallbackData);
-
-private:
-    SList<SListElem<EventPipeProviderCallbackData>> list;
-};
-
 class EventPipeConfiguration
 {
 public: