Make sure EventPipeProvider::m_keywords is accurate after EventPipeConfiguration...

diff --git a/src/vm/eventpipe.cpp b/src/vm/eventpipe.cpp
index c6d23b0..92d2875 100644 (file)
--- a/src/vm/eventpipe.cpp
+++ b/src/vm/eventpipe.cpp
@@ -342,13 +342,6 @@ void EventPipe::DisableInternal(EventPipeSessionID id, EventPipeProviderCallback
     // Disable pSession tracing.
     s_config.Disable(*pSession, pEventPipeProviderCallbackDataQueue);
 
-    // At this point, we should not be writing events to this session anymore
-    // This is a good time to remove the session from the array.
-    _ASSERTE(s_pSessions[pSession->GetIndex()] == pSession);
-
-    // Remove the session from the array, and mask.
-    s_pSessions[pSession->GetIndex()].Store(nullptr);
-
     pSession->Disable(); // Suspend EventPipeBufferManager, and remove providers.
 
     // Do rundown before fully stopping the session.
@@ -375,6 +368,13 @@ void EventPipe::DisableInternal(EventPipeSessionID id, EventPipeProviderCallback
 
     --s_numberOfSessions;
 
+    // At this point, we should not be writing events to this session anymore
+    // This is a good time to remove the session from the array.
+    _ASSERTE(s_pSessions[pSession->GetIndex()] == pSession);
+
+    // Remove the session from the array, and mask.
+    s_pSessions[pSession->GetIndex()].Store(nullptr);
+
     // Write a final sequence point to the file now that all events have
     // been emitted.
     pSession->WriteSequencePointUnbuffered();

diff --git a/src/vm/eventpipeconfiguration.cpp b/src/vm/eventpipeconfiguration.cpp
index 3296691..a1c91e8 100644 (file)
--- a/src/vm/eventpipeconfiguration.cpp
+++ b/src/vm/eventpipeconfiguration.cpp
@@ -159,6 +159,10 @@ bool EventPipeConfiguration::RegisterProvider(EventPipeProvider &provider, Event
         m_pProviderList->InsertTail(new SListElem<EventPipeProvider *>(&provider));
     }
 
+    INT64 keywordForAllSessions;
+    EventPipeEventLevel levelForAllSessions;
+    ComputeKeywordAndLevel(provider, /* out */ keywordForAllSessions, /* out */ levelForAllSessions);
+
     EventPipe::ForEachSession([&](EventPipeSession &session) {
         // Set the provider configuration and enable it if it has been requested by a session.
         EventPipeSessionProvider *pSessionProvider = GetSessionProvider(session, &provider);
@@ -166,6 +170,8 @@ bool EventPipeConfiguration::RegisterProvider(EventPipeProvider &provider, Event
             return;
 
         EventPipeProviderCallbackData eventPipeProviderCallbackData = provider.SetConfiguration(
+            keywordForAllSessions,
+            levelForAllSessions,
             session.GetMask(),
             pSessionProvider->GetKeywords(),
             pSessionProvider->GetLevel(),
@@ -265,7 +271,7 @@ EventPipeProvider *EventPipeConfiguration::GetProviderNoLock(const SString &prov
     return NULL;
 }
 
-EventPipeSessionProvider *EventPipeConfiguration::GetSessionProvider(const EventPipeSession &session, EventPipeProvider *pProvider)
+EventPipeSessionProvider *EventPipeConfiguration::GetSessionProvider(const EventPipeSession &session, const EventPipeProvider *pProvider) const
 {
     CONTRACTL
     {
@@ -279,6 +285,30 @@ EventPipeSessionProvider *EventPipeConfiguration::GetSessionProvider(const Event
     return session.GetSessionProvider(pProvider);
 }
 
+void EventPipeConfiguration::ComputeKeywordAndLevel(const EventPipeProvider& provider, INT64& keywordForAllSessions, EventPipeEventLevel& levelForAllSessions) const
+{
+    CONTRACTL
+    {
+        THROWS;
+        GC_TRIGGERS;
+        MODE_PREEMPTIVE;
+        PRECONDITION(EventPipe::IsLockOwnedByCurrentThread());
+    }
+    CONTRACTL_END;
+    keywordForAllSessions = 0;
+    levelForAllSessions = EventPipeEventLevel::LogAlways;
+    EventPipe::ForEachSession([&](EventPipeSession &session) {
+        EventPipeSessionProvider *pSessionProvider = GetSessionProvider(session, &provider);
+        if (pSessionProvider != nullptr)
+        {
+            INT64 sessionKeyword = pSessionProvider->GetKeywords();
+            EventPipeEventLevel sessionLevel = pSessionProvider->GetLevel();
+            keywordForAllSessions = keywordForAllSessions | sessionKeyword;
+            levelForAllSessions = (sessionLevel > levelForAllSessions) ? sessionLevel : levelForAllSessions;
+        }
+    });
+}
+
 void EventPipeConfiguration::Enable(EventPipeSession &session, EventPipeProviderCallbackDataQueue *pEventPipeProviderCallbackDataQueue)
 {
     CONTRACTL
@@ -302,8 +332,14 @@ void EventPipeConfiguration::Enable(EventPipeSession &session, EventPipeProvider
             EventPipeSessionProvider *pSessionProvider = GetSessionProvider(session, pProvider);
             if (pSessionProvider != NULL)
             {
+                INT64 keywordForAllSessions;
+                EventPipeEventLevel levelForAllSessions;
+                ComputeKeywordAndLevel(*pProvider, /* out */ keywordForAllSessions, /* out */ levelForAllSessions);
+
                 EventPipeProviderCallbackData eventPipeProviderCallbackData =
                     pProvider->SetConfiguration(
+                        keywordForAllSessions,
+                        levelForAllSessions,
                         session.GetMask(),
                         pSessionProvider->GetKeywords(),
                         pSessionProvider->GetLevel(),
@@ -334,13 +370,18 @@ void EventPipeConfiguration::Disable(const EventPipeSession &session, EventPipeP
         while (pElem != NULL)
         {
             EventPipeProvider *pProvider = pElem->GetValue();
-
             if (pProvider->IsEnabled(session.GetMask()))
             {
                 EventPipeSessionProvider *pSessionProvider = GetSessionProvider(session, pProvider);
                 if (pSessionProvider != nullptr)
                 {
+                    INT64 keywordForAllSessions;
+                    EventPipeEventLevel levelForAllSessions;
+                    ComputeKeywordAndLevel(*pProvider, /* out */ keywordForAllSessions, /* out */ levelForAllSessions);
+
                     EventPipeProviderCallbackData eventPipeProviderCallbackData = pProvider->UnsetConfiguration(
+                        keywordForAllSessions,
+                        levelForAllSessions,
                         session.GetMask(),
                         pSessionProvider->GetKeywords(),
                         pSessionProvider->GetLevel(),

diff --git a/src/vm/eventpipeconfiguration.h b/src/vm/eventpipeconfiguration.h
index a9148cb..5f751ef 100644 (file)
--- a/src/vm/eventpipeconfiguration.h
+++ b/src/vm/eventpipeconfiguration.h
@@ -60,7 +60,10 @@ private:
     EventPipeProvider *GetProviderNoLock(const SString &providerID);
 
     // Get the enabled provider.
-    EventPipeSessionProvider *GetSessionProvider(const EventPipeSession &session, EventPipeProvider *pProvider);
+    EventPipeSessionProvider *GetSessionProvider(const EventPipeSession &session, const EventPipeProvider *pProvider) const;
+
+    // Compute the keyword union and maximum level for a provider across all sessions
+    void ComputeKeywordAndLevel(const EventPipeProvider& provider, INT64& keywordsForAllSessions, EventPipeEventLevel& levelForAllSessions) const;
 
     // The list of event pipe providers.
     SList<SListElem<EventPipeProvider *>> *m_pProviderList = nullptr;

diff --git a/src/vm/eventpipeprovider.cpp b/src/vm/eventpipeprovider.cpp
index eb32dd8..e1681f7 100644 (file)
--- a/src/vm/eventpipeprovider.cpp
+++ b/src/vm/eventpipeprovider.cpp
@@ -107,6 +107,8 @@ bool EventPipeProvider::EventEnabled(INT64 keywords, EventPipeEventLevel eventLe
 }
 
 EventPipeProviderCallbackData EventPipeProvider::SetConfiguration(
+    INT64 keywordsForAllSessions,
+    EventPipeEventLevel providerLevelForAllSessions,
     uint64_t sessionMask,
     INT64 keywords,
     EventPipeEventLevel providerLevel,
@@ -124,21 +126,20 @@ EventPipeProviderCallbackData EventPipeProvider::SetConfiguration(
 
     m_sessions |= sessionMask;
 
-    // Set Keywords to be the union of all keywords
-    m_keywords |= keywords;
+    m_keywords = keywordsForAllSessions;
+    m_providerLevel = providerLevelForAllSessions;
 
-    // Set the provider level to "Log Always" or the biggest verbosity.
-    m_providerLevel = (providerLevel < m_providerLevel) ? m_providerLevel : providerLevel;
-
-    RefreshAllEvents(sessionMask, keywords, providerLevel);
+    RefreshAllEvents();
     return PrepareCallbackData(keywords, providerLevel, pFilterData);
 }
 
 EventPipeProviderCallbackData EventPipeProvider::UnsetConfiguration(
-        uint64_t sessionMask,
-        INT64 keywords,
-        EventPipeEventLevel providerLevel,
-        LPCWSTR pFilterData)
+    INT64 keywordsForAllSessions,
+    EventPipeEventLevel providerLevelForAllSessions,
+    uint64_t sessionMask,
+    INT64 keywords,
+    EventPipeEventLevel providerLevel,
+    LPCWSTR pFilterData)
 {
     CONTRACTL
     {
@@ -152,6 +153,11 @@ EventPipeProviderCallbackData EventPipeProvider::UnsetConfiguration(
 
     if (m_sessions & sessionMask)
         m_sessions &= ~sessionMask;
+
+    m_keywords = keywordsForAllSessions;
+    m_providerLevel = providerLevelForAllSessions;
+
+    RefreshAllEvents();
     return PrepareCallbackData(keywords, providerLevel, pFilterData);
 }
 
@@ -293,10 +299,7 @@ void EventPipeProvider::SetDeleteDeferred()
     m_deleteDeferred = true;
 }
 
-void EventPipeProvider::RefreshAllEvents(
-    uint64_t sessionMask,
-    INT64 keywords,
-    EventPipeEventLevel providerLevel)
+void EventPipeProvider::RefreshAllEvents()
 {
     CONTRACTL
     {

diff --git a/src/vm/eventpipeprovider.h b/src/vm/eventpipeprovider.h
index b8ae87e..ad7663d 100644 (file)
--- a/src/vm/eventpipeprovider.h
+++ b/src/vm/eventpipeprovider.h
@@ -89,6 +89,8 @@ private:
     // Set the provider configuration (enable sets of events).
     // This is called by EventPipeConfiguration.
     EventPipeProviderCallbackData SetConfiguration(
+        INT64 keywordsForAllSessions,
+        EventPipeEventLevel providerLevelForAllSessions,
         uint64_t sessionMask,
         INT64 keywords,
         EventPipeEventLevel providerLevel,
@@ -97,16 +99,15 @@ private:
     // Unset the provider configuration for the specified session (disable sets of events).
     // This is called by EventPipeConfiguration.
     EventPipeProviderCallbackData UnsetConfiguration(
+        INT64 keywordsForAllSessions,
+        EventPipeEventLevel providerLevelForAllSessions,
         uint64_t sessionMask,
         INT64 keywords,
         EventPipeEventLevel providerLevel,
         LPCWSTR pFilterData);
 
     // Refresh the runtime state of all events.
-    void RefreshAllEvents(
-        uint64_t sessionMask,
-        INT64 keywords,
-        EventPipeEventLevel providerLevel);
+    void RefreshAllEvents();
 
     // Prepare the data required for invoking callback
     EventPipeProviderCallbackData PrepareCallbackData(

diff --git a/src/vm/eventpipesession.cpp b/src/vm/eventpipesession.cpp
index e7d657b..8123c7e 100644 (file)
--- a/src/vm/eventpipesession.cpp
+++ b/src/vm/eventpipesession.cpp
@@ -259,7 +259,7 @@ void EventPipeSession::AddSessionProvider(EventPipeSessionProvider *pProvider)
     m_pProviderList->AddSessionProvider(pProvider);
 }
 
-EventPipeSessionProvider *EventPipeSession::GetSessionProvider(EventPipeProvider *pProvider) const
+EventPipeSessionProvider *EventPipeSession::GetSessionProvider(const EventPipeProvider *pProvider) const
 {
     CONTRACTL
     {

diff --git a/src/vm/eventpipesession.h b/src/vm/eventpipesession.h
index 9b23403..2777293 100644 (file)
--- a/src/vm/eventpipesession.h
+++ b/src/vm/eventpipesession.h
@@ -176,7 +176,7 @@ public:
     void AddSessionProvider(EventPipeSessionProvider *pProvider);
 
     // Get the session provider for the specified provider if present.
-    EventPipeSessionProvider* GetSessionProvider(EventPipeProvider *pProvider) const;
+    EventPipeSessionProvider* GetSessionProvider(const EventPipeProvider *pProvider) const;
 
     bool WriteAllBuffersToFile();
 

diff --git a/src/vm/eventpipesessionprovider.cpp b/src/vm/eventpipesessionprovider.cpp
index 5a54166..0120a62 100644 (file)
--- a/src/vm/eventpipesessionprovider.cpp
+++ b/src/vm/eventpipesessionprovider.cpp
@@ -137,7 +137,7 @@ void EventPipeSessionProviderList::AddSessionProvider(EventPipeSessionProvider *
         m_pProviders->InsertTail(new SListElem<EventPipeSessionProvider *>(pProvider));
 }
 
-EventPipeSessionProvider *EventPipeSessionProviderList::GetSessionProvider(EventPipeProvider *pProvider) const
+EventPipeSessionProvider *EventPipeSessionProviderList::GetSessionProvider(const EventPipeProvider *pProvider) const
 {
     CONTRACTL
     {

diff --git a/src/vm/eventpipesessionprovider.h b/src/vm/eventpipesessionprovider.h
index 7a6dafa..642cf4f 100644 (file)
--- a/src/vm/eventpipesessionprovider.h
+++ b/src/vm/eventpipesessionprovider.h
@@ -57,7 +57,7 @@ public:
 
     // Get the session provider for the specified provider.
     // Return NULL if one doesn't exist.
-    EventPipeSessionProvider* GetSessionProvider(EventPipeProvider *pProvider) const;
+    EventPipeSessionProvider* GetSessionProvider(const EventPipeProvider *pProvider) const;
 
     // Returns true if the list is empty.
     bool IsEmpty() const;