From: Andrew Au Date: Tue, 25 Jun 2019 20:36:18 +0000 (-0700) Subject: Make sure EventPipeProvider::m_keywords is accurate after EventPipeConfiguration... X-Git-Tag: accepted/tizen/unified/20190813.215958~40^2~22 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=86fcae86d32d6b7117a7f7ca6b4788eb1759c867;p=platform%2Fupstream%2Fcoreclr.git Make sure EventPipeProvider::m_keywords is accurate after EventPipeConfiguration::Disable (#25358) --- diff --git a/src/vm/eventpipe.cpp b/src/vm/eventpipe.cpp index c6d23b0..92d2875 100644 --- 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 --- 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(&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 --- 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> *m_pProviderList = nullptr; diff --git a/src/vm/eventpipeprovider.cpp b/src/vm/eventpipeprovider.cpp index eb32dd8..e1681f7 100644 --- 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 --- 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 --- 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 --- 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 --- a/src/vm/eventpipesessionprovider.cpp +++ b/src/vm/eventpipesessionprovider.cpp @@ -137,7 +137,7 @@ void EventPipeSessionProviderList::AddSessionProvider(EventPipeSessionProvider * m_pProviders->InsertTail(new SListElem(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 --- 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;