From: Brian Robbins Date: Wed, 17 May 2017 00:11:47 +0000 (-0700) Subject: Allow provider deletion to be deferred until after tracing is stopped. (#11651) X-Git-Tag: accepted/tizen/base/20180629.140029~1109^2~11 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=40b87c9810c31fb6ab471a44b5735408269ce6ba;p=platform%2Fupstream%2Fcoreclr.git 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 --- 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 --- 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(provHandle); - delete pProvider; + EventPipe::DeleteProvider(pProvider); } END_QCALL; diff --git a/src/vm/eventpipe.h b/src/vm/eventpipe.h index 0aced25..092508f 100644 --- 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 --- 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 *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 --- 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..896f9b26 100644 --- 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 --- 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 --- 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 */