diff --git a/src/coreclr/src/vm/eventpipe.cpp b/src/coreclr/src/vm/eventpipe.cpp index 4c5918ffc2032..debdcdd2de52a 100644 --- a/src/coreclr/src/vm/eventpipe.cpp +++ b/src/coreclr/src/vm/eventpipe.cpp @@ -1041,9 +1041,9 @@ HANDLE EventPipe::GetWaitHandle(EventPipeSessionID sessionID) return pSession ? pSession->GetWaitEvent()->GetHandleUNHOSTED() : 0; } -void EventPipe::InvokeCallback(EventPipeProviderCallbackData eventPipeProviderCallbackData) +void EventPipe::InvokeCallback(EventPipeProviderCallbackData *pEventPipeProviderCallbackData) { - EventPipeProvider::InvokeCallback(eventPipeProviderCallbackData); + EventPipeProvider::InvokeCallback(pEventPipeProviderCallbackData); } EventPipeEventInstance *EventPipe::BuildEventMetadataEvent(EventPipeEventInstance &instance, unsigned int metadataId) diff --git a/src/coreclr/src/vm/eventpipe.h b/src/coreclr/src/vm/eventpipe.h index 52e7061d0223c..ae56b9c771e5e 100644 --- a/src/coreclr/src/vm/eventpipe.h +++ b/src/coreclr/src/vm/eventpipe.h @@ -139,7 +139,7 @@ class EventPipe } while (eventPipeProviderCallbackDataQueue.TryDequeue(&eventPipeProviderCallbackData)) - InvokeCallback(eventPipeProviderCallbackData); + InvokeCallback(&eventPipeProviderCallbackData); } // Returns the a number 0...N representing the processor number this thread is currently @@ -158,7 +158,7 @@ class EventPipe } private: - static void InvokeCallback(EventPipeProviderCallbackData eventPipeProviderCallbackData); + static void InvokeCallback(EventPipeProviderCallbackData *pEventPipeProviderCallbackData); // Get the event used to write metadata to the event stream. static EventPipeEventInstance *BuildEventMetadataEvent(EventPipeEventInstance &instance, unsigned int metadataId); diff --git a/src/coreclr/src/vm/eventpipecommontypes.cpp b/src/coreclr/src/vm/eventpipecommontypes.cpp index e668dc1aa2096..f43b4873a6e45 100644 --- a/src/coreclr/src/vm/eventpipecommontypes.cpp +++ b/src/coreclr/src/vm/eventpipecommontypes.cpp @@ -8,8 +8,7 @@ void EventPipeProviderCallbackDataQueue::Enqueue(EventPipeProviderCallbackData *pEventPipeProviderCallbackData) { - SListElem *listnode = new SListElem(); // throws - listnode->m_Value = *pEventPipeProviderCallbackData; + SListElem *listnode = new SListElem(std::move(*pEventPipeProviderCallbackData)); // throws list.InsertTail(listnode); } @@ -19,7 +18,7 @@ bool EventPipeProviderCallbackDataQueue::TryDequeue(EventPipeProviderCallbackDat return false; SListElem *listnode = list.RemoveHead(); - *pEventPipeProviderCallbackData = listnode->m_Value; + *pEventPipeProviderCallbackData = std::move(listnode->m_Value); delete listnode; return true; } diff --git a/src/coreclr/src/vm/eventpipecommontypes.h b/src/coreclr/src/vm/eventpipecommontypes.h index 1a66034adb87e..5bb4a1894a066 100644 --- a/src/coreclr/src/vm/eventpipecommontypes.h +++ b/src/coreclr/src/vm/eventpipecommontypes.h @@ -98,20 +98,129 @@ typedef void (*EventPipeCallback)( EventFilterDescriptor *FilterData, void *CallbackContext); -struct EventPipeProviderCallbackData +class EventPipeProviderCallbackData { - LPCWSTR pFilterData; - EventPipeCallback pCallbackFunction; - bool enabled; - INT64 keywords; - EventPipeEventLevel providerLevel; - void* pCallbackData; +public: + EventPipeProviderCallbackData(): + m_pFilterData(nullptr), + m_pCallbackFunction(nullptr), + m_enabled(false), + m_keywords(0), + m_providerLevel(EventPipeEventLevel::LogAlways), + m_pCallbackData(nullptr), + m_pProvider(nullptr) + { + + } + + EventPipeProviderCallbackData(LPCWSTR pFilterData, + EventPipeCallback pCallbackFunction, + bool enabled, + INT64 keywords, + EventPipeEventLevel providerLevel, + void* pCallbackData, + EventPipeProvider *pProvider) : + m_pFilterData(nullptr), + m_pCallbackFunction(pCallbackFunction), + m_enabled(enabled), + m_keywords(keywords), + m_providerLevel(providerLevel), + m_pCallbackData(pCallbackData), + m_pProvider(pProvider) + { + if (pFilterData != nullptr) + { + // This is the only way to create an EventPipeProviderCallbackData that will copy the + // filter data. The copying is intentional, because sessions die before callbacks happen + // so we cannot cache a pointer to the session's filter data. + size_t bufSize = wcslen(pFilterData) + 1; + m_pFilterData = new WCHAR[bufSize]; + wcscpy_s(m_pFilterData, bufSize, pFilterData); + } + } + + EventPipeProviderCallbackData(EventPipeProviderCallbackData &&other) + : EventPipeProviderCallbackData() + { + *this = std::move(other); + } + + EventPipeProviderCallbackData &operator=(EventPipeProviderCallbackData &&other) + { + std::swap(m_pFilterData, other.m_pFilterData); + m_pCallbackFunction = other.m_pCallbackFunction; + m_enabled = other.m_enabled; + m_keywords = other.m_keywords; + m_providerLevel = other.m_providerLevel; + m_pCallbackData = other.m_pCallbackData; + m_pProvider = other.m_pProvider; + + return *this; + } + + // We don't want to be unintentionally copying and deleting the filter data any more + // than we have to. Moving (above) is fine, but copying should be avoided. + EventPipeProviderCallbackData(const EventPipeProviderCallbackData &other) = delete; + EventPipeProviderCallbackData &operator=(const EventPipeProviderCallbackData &other) = delete; + + ~EventPipeProviderCallbackData() + { + if (m_pFilterData != nullptr) + { + delete[] m_pFilterData; + m_pFilterData = nullptr; + } + } + + LPCWSTR GetFilterData() const + { + return m_pFilterData; + } + + EventPipeCallback GetCallbackFunction() const + { + return m_pCallbackFunction; + } + + bool GetEnabled() const + { + return m_enabled; + } + + INT64 GetKeywords() const + { + return m_keywords; + } + + EventPipeEventLevel GetProviderLevel() const + { + return m_providerLevel; + } + + void *GetCallbackData() const + { + return m_pCallbackData; + } + + EventPipeProvider *GetProvider() const + { + return m_pProvider; + } + +private: + WCHAR *m_pFilterData; + EventPipeCallback m_pCallbackFunction; + bool m_enabled; + INT64 m_keywords; + EventPipeEventLevel m_providerLevel; + void* m_pCallbackData; + EventPipeProvider *m_pProvider; }; class EventPipeProviderCallbackDataQueue { public: - void Enqueue(EventPipeProviderCallbackData* pEventPipeProviderCallbackData); + void Enqueue(EventPipeProviderCallbackData *pEventPipeProviderCallbackData); bool TryDequeue(EventPipeProviderCallbackData* pEventPipeProviderCallbackData); private: diff --git a/src/coreclr/src/vm/eventpipeprovider.cpp b/src/coreclr/src/vm/eventpipeprovider.cpp index 7aaa31f887b14..916d7c55a1fa4 100644 --- a/src/coreclr/src/vm/eventpipeprovider.cpp +++ b/src/coreclr/src/vm/eventpipeprovider.cpp @@ -194,7 +194,7 @@ void EventPipeProvider::AddEvent(EventPipeEvent &event) event.RefreshState(); } -/* static */ void EventPipeProvider::InvokeCallback(EventPipeProviderCallbackData eventPipeProviderCallbackData) +/* static */ void EventPipeProvider::InvokeCallback(EventPipeProviderCallbackData *pEventPipeProviderCallbackData) { CONTRACTL { @@ -205,12 +205,12 @@ void EventPipeProvider::AddEvent(EventPipeEvent &event) } CONTRACTL_END; - LPCWSTR pFilterData = eventPipeProviderCallbackData.pFilterData; - EventPipeCallback pCallbackFunction = eventPipeProviderCallbackData.pCallbackFunction; - bool enabled = eventPipeProviderCallbackData.enabled; - INT64 keywords = eventPipeProviderCallbackData.keywords; - EventPipeEventLevel providerLevel = eventPipeProviderCallbackData.providerLevel; - void *pCallbackData = eventPipeProviderCallbackData.pCallbackData; + LPCWSTR pFilterData = pEventPipeProviderCallbackData->GetFilterData(); + EventPipeCallback pCallbackFunction = pEventPipeProviderCallbackData->GetCallbackFunction(); + bool enabled = pEventPipeProviderCallbackData->GetEnabled(); + INT64 keywords = pEventPipeProviderCallbackData->GetKeywords(); + EventPipeEventLevel providerLevel = pEventPipeProviderCallbackData->GetProviderLevel(); + void *pCallbackData = pEventPipeProviderCallbackData->GetCallbackData(); bool isEventFilterDescriptorInitialized = false; EventFilterDescriptor eventFilterDescriptor{}; @@ -286,13 +286,13 @@ EventPipeProviderCallbackData EventPipeProvider::PrepareCallbackData( } CONTRACTL_END; - EventPipeProviderCallbackData result; - result.pFilterData = pFilterData; - result.pCallbackFunction = m_pCallbackFunction; - result.enabled = (m_sessions != 0); - result.providerLevel = providerLevel; - result.keywords = keywords; - result.pCallbackData = m_pCallbackData; + EventPipeProviderCallbackData result(pFilterData, + m_pCallbackFunction, + (m_sessions != 0), + keywords, + providerLevel, + m_pCallbackData, + this); return result; } diff --git a/src/coreclr/src/vm/eventpipeprovider.h b/src/coreclr/src/vm/eventpipeprovider.h index fab3bc75fcf44..864c53d00b282 100644 --- a/src/coreclr/src/vm/eventpipeprovider.h +++ b/src/coreclr/src/vm/eventpipeprovider.h @@ -113,7 +113,7 @@ class EventPipeProvider LPCWSTR pFilterData); // Invoke the provider callback. - static void InvokeCallback(EventPipeProviderCallbackData eventPipeProviderCallbackData); + static void InvokeCallback(EventPipeProviderCallbackData *pEventPipeProviderCallbackData); // Specifies whether or not the provider was deleted, but that deletion // was deferred until after tracing is stopped.