Multiple bug fixes (#25308)

- Fixes EventPipe does not properly dispose of itself on an active session error #25228
  - On EventPipeSession::ThreadProc, if an error occurs on the IPC streaming, then EventPipe::Disable will be invoked.
- Fixes Prevent EventPipe Sessions IDs from being reused on disconnect #25229

14 files changed, 196 insertions, 288 deletions

diff --git a/src/vm/eventpipe.cpp b/src/vm/eventpipe.cpp
index 81b588f485..fc29160d3e 100644
--- a/src/vm/eventpipe.cpp
+++ b/src/vm/eventpipe.cpp
@@ -29,20 +29,20 @@
 #ifdef FEATURE_PERFTRACING
 
 CrstStatic EventPipe::s_configCrst;
-Volatile<bool> EventPipe::s_tracingInitialized = false;
+Volatile<bool> EventPipe::s_tracingInitialized(false);
 EventPipeConfiguration EventPipe::s_config;
 EventPipeEventSource *EventPipe::s_pEventSource = nullptr;
 VolatilePtr<EventPipeSession> EventPipe::s_pSessions[MaxNumberOfSessions];
 #ifndef FEATURE_PAL
 unsigned int * EventPipe::s_pProcGroupOffsets = nullptr;
 #endif
+uint32_t EventPipe::s_numberOfSessions = 0;
 
-#ifdef FEATURE_PAL
 // This function is auto-generated from /src/scripts/genEventPipe.py
-extern "C" void InitProvidersAndEvents();
-#else
-void InitProvidersAndEvents();
+#ifdef FEATURE_PAL
+extern "C"
 #endif
+void InitProvidersAndEvents();
 
 void EventPipe::Initialize()
 {
@@ -189,7 +189,12 @@ EventPipeSessionID EventPipe::Enable(
         if (!s_tracingInitialized)
             return;
 
-        EventPipeSession *const pSession = s_config.CreateSession(
+        const uint32_t SessionIndex = GenerateSessionIndex();
+        if (SessionIndex >= EventPipe::MaxNumberOfSessions)
+            return;
+
+        EventPipeSession *const pSession = new EventPipeSession(
+            SessionIndex,
             strOutputPath,
             pStream,
             sessionType,
@@ -198,25 +203,17 @@ EventPipeSessionID EventPipe::Enable(
             pProviders,
             numProviders);
 
-        if (pSession == nullptr)
-            return;
-        sessionId = EnableInternal(pSession, pEventPipeProviderCallbackDataQueue);
-        if (sessionId == 0)
+        const bool fSuccess = EnableInternal(pSession, pEventPipeProviderCallbackDataQueue);
+        if (fSuccess)
+            sessionId = reinterpret_cast<EventPipeSessionID>(pSession);
+        else
             delete pSession;
     });
 
     return sessionId;
 }
 
-static uint64_t GetArrayIndex(EventPipeSessionID mask)
-{
-    for (uint64_t i = 0; i < 64; ++i)
-        if ((1ULL << i) & mask)
-            return i;
-    return UINT64_MAX;
-}
-
-EventPipeSessionID EventPipe::EnableInternal(
+bool EventPipe::EnableInternal(
     EventPipeSession *const pSession,
     EventPipeProviderCallbackDataQueue* pEventPipeProviderCallbackDataQueue)
 {
@@ -232,14 +229,19 @@ EventPipeSessionID EventPipe::EnableInternal(
     CONTRACTL_END;
 
     if (pSession == nullptr || !pSession->IsValid())
-        return 0;
+        return false;
 
     // Return if the index is invalid.
-    const uint64_t index = GetArrayIndex(pSession->GetId());
-    if (index >= 64)
+    if (pSession->GetIndex() >= MaxNumberOfSessions)
     {
-        _ASSERTE(!"Computed index was out of range.");
-        return 0;
+        _ASSERTE(!"Session index was out of range.");
+        return false;
+    }
+
+    if (s_numberOfSessions > MaxNumberOfSessions)
+    {
+        _ASSERTE(!"Max number of sessions reached.");
+        return false;
     }
 
     // Register the SampleProfiler the very first time.
@@ -249,12 +251,13 @@ EventPipeSessionID EventPipe::EnableInternal(
     s_pEventSource->Enable(pSession);
 
     // Save the session.
-    if (s_pSessions[index].LoadWithoutBarrier() != nullptr)
+    if (s_pSessions[pSession->GetIndex()].LoadWithoutBarrier() != nullptr)
     {
         _ASSERTE(!"Attempting to override an existing session.");
-        return 0;
+        return false;
     }
-    s_pSessions[index].Store(pSession);
+    s_pSessions[pSession->GetIndex()].Store(pSession);
+    ++s_numberOfSessions;
 
     // Enable tracing.
     s_config.Enable(*pSession, pEventPipeProviderCallbackDataQueue);
@@ -265,8 +268,7 @@ EventPipeSessionID EventPipe::EnableInternal(
     // Enable the session.
     pSession->Enable();
 
-    // Return the session ID.
-    return pSession->GetId();
+    return true;
 }
 
 void EventPipe::Disable(EventPipeSessionID id)
@@ -288,7 +290,8 @@ void EventPipe::Disable(EventPipeSessionID id)
     GCX_PREEMP();
 
     RunWithCallbackPostponed([&](EventPipeProviderCallbackDataQueue *pEventPipeProviderCallbackDataQueue) {
-        DisableInternal(id, pEventPipeProviderCallbackDataQueue);
+        if (s_numberOfSessions > 0)
+            DisableInternal(id, pEventPipeProviderCallbackDataQueue);
     });
 }
 
@@ -315,25 +318,17 @@ void EventPipe::DisableInternal(EventPipeSessionID id, EventPipeProviderCallback
         THROWS;
         GC_TRIGGERS;
         MODE_ANY;
+        PRECONDITION(id != 0);
+        PRECONDITION(s_numberOfSessions > 0);
         PRECONDITION(IsLockOwnedByCurrentThread());
     }
     CONTRACTL_END;
 
-    if (!s_config.Enabled() || !s_config.IsSessionIdValid(id))
-        return;
-
-    // Get the specified session ID.
-    const uint64_t index = GetArrayIndex(id);
-    if (index >= 64)
-    {
-        _ASSERTE(!"Computed index was out of range.");
+    if (!Enabled() || !IsSessionIdInCollection(id))
         return;
-    }
 
     // If the session was not found, then there is nothing else to do.
-    EventPipeSession *const pSession = s_pSessions[index];
-    if (pSession == nullptr)
-        return;
+    EventPipeSession *const pSession = reinterpret_cast<EventPipeSession *>(id);
 
     // Disable the profiler.
     SampleProfiler::Disable();
@@ -346,7 +341,10 @@ void EventPipe::DisableInternal(EventPipeSessionID id, EventPipeProviderCallback
 
     // 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.
-    s_pSessions[index].Store(nullptr);
+    _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.
 
@@ -359,7 +357,9 @@ void EventPipe::DisableInternal(EventPipeSessionID id, EventPipeProviderCallback
         pEventPipeThread->SetAsRundownThread(pSession);
         {
             s_config.Enable(*pSession, pEventPipeProviderCallbackDataQueue);
-            pSession->ExecuteRundown();
+            {
+                pSession->ExecuteRundown();
+            }
             s_config.Disable(*pSession, pEventPipeProviderCallbackDataQueue);
         }
         pEventPipeThread->SetAsRundownThread(nullptr);
@@ -370,14 +370,14 @@ void EventPipe::DisableInternal(EventPipeSessionID id, EventPipeProviderCallback
         return;
     }
 
+    --s_numberOfSessions;
+
     // Write a final sequence point to the file now that all events have
-    // been emitted
+    // been emitted.
     pSession->WriteSequencePointUnbuffered();
 
-    // Remove the session.
-    s_config.DeleteSession(pSession);
+    delete pSession;
 
-    // Delete deferred providers.
     // Providers can't be deleted during tracing because they may be needed when serializing the file.
     s_config.DeleteDeferredProviders();
 }
@@ -395,24 +395,11 @@ EventPipeSession *EventPipe::GetSession(EventPipeSessionID id)
             return nullptr;
         }
 
-        // Attempt to get the specified session ID.
-        const uint64_t index = GetArrayIndex(id);
-        if (index >= 64)
-        {
-            _ASSERTE(!"Computed index was out of range.");
-            return nullptr;
-        }
-
-        return s_pSessions[index];
+        return IsSessionIdInCollection(id) ?
+            reinterpret_cast<EventPipeSession*>(id) : nullptr;
     }
 }
 
-bool EventPipe::Enabled()
-{
-    LIMITED_METHOD_CONTRACT;
-    return s_tracingInitialized && s_config.Enabled();
-}
-
 EventPipeProvider *EventPipe::CreateProvider(const SString &providerName, EventPipeCallback pCallbackFunction, void *pCallbackData)
 {
     CONTRACTL
@@ -623,7 +610,7 @@ void EventPipe::WriteEventInternal(
     }
     else
     {
-        for (uint64_t i = 0; i < MaxNumberOfSessions; ++i)
+        for (uint32_t i = 0; i < MaxNumberOfSessions; ++i)
         {
             // This read is OK because we aren't derefencing the pointer and if we observe a value that
             // isn't up-to-date (whether null or non-null) that is just the natural race timing of trying to
@@ -655,7 +642,7 @@ void EventPipe::WriteEventInternal(
             }
             // Do not reference pSession past this point, we are signaling Disable() that it is safe to
             // delete it
-            pEventPipeThread->SetSessionWriteInProgress(UINT64_MAX);
+            pEventPipeThread->SetSessionWriteInProgress(UINT32_MAX);
         }
     }
 }
@@ -756,6 +743,35 @@ StackWalkAction EventPipe::StackWalkCallback(CrawlFrame *pCf, StackContents *pDa
     return SWA_CONTINUE;
 }
 
+uint32_t EventPipe::GenerateSessionIndex()
+{
+    LIMITED_METHOD_CONTRACT;
+    PRECONDITION(IsLockOwnedByCurrentThread());
+
+    for (uint32_t i = 0; i < MaxNumberOfSessions; ++i)
+        if (s_pSessions[i].LoadWithoutBarrier() == nullptr)
+            return i;
+    return MaxNumberOfSessions;
+}
+
+bool EventPipe::IsSessionIdInCollection(EventPipeSessionID id)
+{
+    LIMITED_METHOD_CONTRACT;
+    PRECONDITION(id != 0);
+    PRECONDITION(IsLockOwnedByCurrentThread());
+
+    const EventPipeSession *const pSession = reinterpret_cast<EventPipeSession *>(id);
+    for (uint32_t i = 0; i < MaxNumberOfSessions; ++i)
+    {
+        if (s_pSessions[i] == pSession)
+        {
+            _ASSERTE(i == pSession->GetIndex());
+            return true;
+        }
+    }
+    return false;
+}
+
 EventPipeEventInstance *EventPipe::GetNextEvent(EventPipeSessionID sessionID)
 {
     CONTRACTL
diff --git a/src/vm/eventpipe.h b/src/vm/eventpipe.h
index 7a8df26343..4089d9003f 100644
--- a/src/vm/eventpipe.h
+++ b/src/vm/eventpipe.h
@@ -60,7 +60,11 @@ public:
     static EventPipeSession *GetSession(EventPipeSessionID id);
 
     // Specifies whether or not the event pipe is enabled.
-    static bool Enabled();
+    static bool Enabled()
+    {
+        LIMITED_METHOD_CONTRACT;
+        return s_tracingInitialized && (s_numberOfSessions > 0);
+    }
 
     // Create a provider.
     static EventPipeProvider *CreateProvider(
@@ -154,13 +158,19 @@ private:
     static void DisableInternal(EventPipeSessionID id, EventPipeProviderCallbackDataQueue* pEventPipeProviderCallbackDataQueue);
 
     // Enable the specified EventPipe session.
-    static EventPipeSessionID EnableInternal(
+    static bool EnableInternal(
         EventPipeSession *const pSession,
         EventPipeProviderCallbackDataQueue *pEventPipeProviderCallbackDataQueue);
 
     // Callback function for the stack walker.  For each frame walked, this callback is invoked.
     static StackWalkAction StackWalkCallback(CrawlFrame *pCf, StackContents *pData);
 
+    //! Helper function used to locate a free index in the range 0 - EventPipe::MaxNumberOfSessions
+    //! Returns EventPipe::MaxNumberOfSessions if there are no free indexes
+    static uint32_t GenerateSessionIndex();
+
+    static bool IsSessionIdInCollection(EventPipeSessionID id);
+
     template <typename EventPipeSessionHandlerCallback>
     static void ForEachSession(EventPipeSessionHandlerCallback callback)
     {
@@ -190,12 +200,13 @@ private:
     static VolatilePtr<EventPipeSession> s_pSessions[MaxNumberOfSessions];
     static EventPipeEventSource *s_pEventSource;
 
-    // A mapping from windows processor group index to the total number of processors
-    // in all groups preceding it. For example if there are three groups with sizes: 
+    //! Bitmask tracking EventPipe active sessions.
+    // in all groups preceding it. For example if there are three groups with sizes:
     // 1, 7, 6 the table would be 0, 1, 8
 #ifndef FEATURE_PAL
     static unsigned int * s_pProcGroupOffsets;
 #endif
+    static uint32_t s_numberOfSessions;
 };
 
 static_assert(EventPipe::MaxNumberOfSessions == 64, "Maximum number of EventPipe sessions is not 64.");
diff --git a/src/vm/eventpipebuffermanager.cpp b/src/vm/eventpipebuffermanager.cpp
index e8f4c5a051..3ac90fd719 100644
--- a/src/vm/eventpipebuffermanager.cpp
+++ b/src/vm/eventpipebuffermanager.cpp
@@ -939,7 +939,7 @@ bool EventPipeBufferManager::TryConvertBufferToReadOnly(EventPipeBuffer* pNewRea
     {
         EventPipeThread* pThread = pNewReadBuffer->GetWriterThread();
         SpinLockHolder _slh(pThread->GetLock());
-        EventPipeThreadSessionState* pSessionState = pThread->GetSessionState(m_pSession);
+        EventPipeThreadSessionState *const pSessionState = pThread->GetSessionState(m_pSession);
         if (pSessionState->GetWriteBuffer() == pNewReadBuffer)
         {
             pSessionState->SetWriteBuffer(nullptr);
@@ -957,7 +957,7 @@ bool EventPipeBufferManager::TryConvertBufferToReadOnly(EventPipeBuffer* pNewRea
     return pNewReadBuffer->GetVolatileState() == EventPipeBufferState::READ_ONLY;
 }
 
-void EventPipeBufferManager::SuspendWriteEvent(EventPipeSessionID sessionId)
+void EventPipeBufferManager::SuspendWriteEvent(uint32_t sessionIndex)
 {
     CONTRACTL
     {
@@ -1000,7 +1000,7 @@ void EventPipeBufferManager::SuspendWriteEvent(EventPipeSessionID sessionId)
         EventPipeThread *pThread = threadList[i];
         {
             SpinLockHolder _slh(pThread->GetLock());
-            EventPipeThreadSessionState* pSessionState = pThread->GetSessionState(m_pSession);
+            EventPipeThreadSessionState *const pSessionState = pThread->GetSessionState(m_pSession);
             pSessionState->SetWriteBuffer(nullptr);
             // From this point until m_writeEventSuspending is reset to FALSE it is impossible
             // for this thread to set the write buffer to a non-null value which in turn means
@@ -1028,7 +1028,7 @@ void EventPipeBufferManager::SuspendWriteEvent(EventPipeSessionID sessionId)
                 EventPipeThread *const pEventPipeThread = pBufferList->GetThread();
                 if (pEventPipeThread != nullptr)
                 {
-                    YIELD_WHILE(pEventPipeThread->GetSessionWriteInProgress() == sessionId);
+                    YIELD_WHILE(pEventPipeThread->GetSessionWriteInProgress() == sessionIndex);
                     // It still guarantees that the thread has returned its buffer, but it also now guarantees that
                     // that the thread has returned from Session::WriteEvent() and has relinquished the session pointer
                     // This yield is guaranteed to eventually finish because threads will eventually exit WriteEvent()
diff --git a/src/vm/eventpipebuffermanager.h b/src/vm/eventpipebuffermanager.h
index dce4177622..9bed01db97 100644
--- a/src/vm/eventpipebuffermanager.h
+++ b/src/vm/eventpipebuffermanager.h
@@ -120,14 +120,14 @@ private:
     // Initially the iterator starts uninitialized and GetCurrentEvent() returns NULL. Calling MoveNextXXX()
     // attempts to advance the cursor to the next event. If there is no event prior to stopTimeStamp then
     // the GetCurrentEvent() again returns NULL, otherwise it returns that event. The event pointer returned
-    // by GetCurrentEvent() is valid until MoveNextXXX() is called again. Once all events in a buffer have 
+    // by GetCurrentEvent() is valid until MoveNextXXX() is called again. Once all events in a buffer have
     // been read the iterator will delete that buffer from the pool.
 
     // Moves to the next oldest event searching across all threads. If there is no event older than
     // stopTimeStamp then GetCurrentEvent() will return NULL.
     void MoveNextEventAnyThread(LARGE_INTEGER stopTimeStamp);
 
-    // Moves to the next oldest event from the same thread as the current event. If there is no event 
+    // Moves to the next oldest event from the same thread as the current event. If there is no event
     // older than stopTimeStamp then GetCurrentEvent() will return NULL. This should only be called
     // when GetCurrentEvent() is non-null (because we need to know what thread's events to iterate)
     void MoveNextEventSameThread(LARGE_INTEGER stopTimeStamp);
@@ -169,7 +169,7 @@ public:
     // EXPECTED USAGE: First the caller will disable all events via configuration, then call
     // SuspendWriteEvent() to force any WriteEvent calls that may still be in progress to either
     // finish or cancel. After that all BufferLists and Buffers can be safely drained and/or deleted.
-    void SuspendWriteEvent(EventPipeSessionID sessionId);
+    void SuspendWriteEvent(uint32_t sessionIndex);
 
     // Write the contents of the managed buffers to the specified file.
     // The stopTimeStamp is used to determine when tracing was stopped to ensure that we
@@ -212,7 +212,7 @@ private:
     unsigned int m_bufferCount;
 
     // The sequence number of the last event that was read, only
-    // updated/read by the reader thread. 
+    // updated/read by the reader thread.
     unsigned int m_lastReadSequenceNumber;
 
 public:
diff --git a/src/vm/eventpipeconfiguration.cpp b/src/vm/eventpipeconfiguration.cpp
index b6c9f01df1..3296691a1d 100644
--- a/src/vm/eventpipeconfiguration.cpp
+++ b/src/vm/eventpipeconfiguration.cpp
@@ -24,7 +24,6 @@ void EventPipeConfiguration::Initialize()
         PRECONDITION(m_pProviderList == nullptr);
         PRECONDITION(m_pConfigProvider == nullptr);
         PRECONDITION(m_pMetadataEvent == nullptr);
-        PRECONDITION(m_activeSessions == 0);
     }
     CONTRACTL_END;
 
@@ -51,7 +50,6 @@ void EventPipeConfiguration::Shutdown()
         NOTHROW;
         GC_TRIGGERS;
         MODE_ANY;
-        PRECONDITION(m_activeSessions == 0);
     }
     CONTRACTL_END;
 
@@ -168,7 +166,7 @@ bool EventPipeConfiguration::RegisterProvider(EventPipeProvider &provider, Event
             return;
 
         EventPipeProviderCallbackData eventPipeProviderCallbackData = provider.SetConfiguration(
-            session.GetId(),
+            session.GetMask(),
             pSessionProvider->GetKeywords(),
             pSessionProvider->GetLevel(),
             pSessionProvider->GetFilterData());
@@ -281,56 +279,6 @@ EventPipeSessionProvider *EventPipeConfiguration::GetSessionProvider(const Event
     return session.GetSessionProvider(pProvider);
 }
 
-EventPipeSession *EventPipeConfiguration::CreateSession(
-    LPCWSTR strOutputPath,
-    IpcStream *const pStream,
-    EventPipeSessionType sessionType,
-    EventPipeSerializationFormat format,
-    unsigned int circularBufferSizeInMB,
-    const EventPipeProviderConfiguration *pProviders,
-    uint32_t numProviders,
-    bool rundownEnabled)
-{
-    CONTRACTL
-    {
-        THROWS;
-        GC_TRIGGERS;
-        MODE_PREEMPTIVE;
-        PRECONDITION(format < EventPipeSerializationFormat::Count);
-        PRECONDITION(circularBufferSizeInMB > 0);
-        PRECONDITION(numProviders > 0 && pProviders != nullptr);
-        PRECONDITION(EventPipe::IsLockOwnedByCurrentThread());
-    }
-    CONTRACTL_END;
-
-    const unsigned int index = GenerateSessionIndex();
-    if (index >= EventPipe::MaxNumberOfSessions)
-    {
-        return nullptr;
-    }
-    return new EventPipeSession(index, strOutputPath, pStream, sessionType, format, circularBufferSizeInMB, pProviders, numProviders);
-}
-
-void EventPipeConfiguration::DeleteSession(EventPipeSession *pSession)
-{
-    CONTRACTL
-    {
-        THROWS;
-        GC_TRIGGERS;
-        MODE_PREEMPTIVE;
-        PRECONDITION(pSession != nullptr);
-        PRECONDITION(EventPipe::IsLockOwnedByCurrentThread());
-    }
-    CONTRACTL_END;
-
-    if (pSession != nullptr)
-    {
-        // Reset the mask of active sessions.
-        m_activeSessions &= ~pSession->GetId();
-        delete pSession;
-    }
-}
-
 void EventPipeConfiguration::Enable(EventPipeSession &session, EventPipeProviderCallbackDataQueue *pEventPipeProviderCallbackDataQueue)
 {
     CONTRACTL
@@ -342,10 +290,6 @@ void EventPipeConfiguration::Enable(EventPipeSession &session, EventPipeProvider
     }
     CONTRACTL_END;
 
-    // Add session Id to the "list" of active sessions.
-    m_activeSessions |= session.GetId();
-    _ASSERTE(IsSessionIdValid(session.GetId()));
-
     // The provider list should be non-NULL, but can be NULL on shutdown.
     if (m_pProviderList != NULL)
     {
@@ -360,7 +304,7 @@ void EventPipeConfiguration::Enable(EventPipeSession &session, EventPipeProvider
             {
                 EventPipeProviderCallbackData eventPipeProviderCallbackData =
                     pProvider->SetConfiguration(
-                        session.GetId(),
+                        session.GetMask(),
                         pSessionProvider->GetKeywords(),
                         pSessionProvider->GetLevel(),
                         pSessionProvider->GetFilterData());
@@ -379,8 +323,6 @@ void EventPipeConfiguration::Disable(const EventPipeSession &session, EventPipeP
         THROWS;
         GC_TRIGGERS;
         MODE_PREEMPTIVE;
-        PRECONDITION((session.GetId() & m_activeSessions) != 0); // Session is enabled.
-        // Lock must be held by EventPipe::Disable.
         PRECONDITION(EventPipe::IsLockOwnedByCurrentThread());
     }
     CONTRACTL_END;
@@ -393,13 +335,13 @@ void EventPipeConfiguration::Disable(const EventPipeSession &session, EventPipeP
         {
             EventPipeProvider *pProvider = pElem->GetValue();
 
-            if (pProvider->IsEnabled(session.GetId()))
+            if (pProvider->IsEnabled(session.GetMask()))
             {
                 EventPipeSessionProvider *pSessionProvider = GetSessionProvider(session, pProvider);
                 if (pSessionProvider != nullptr)
                 {
                     EventPipeProviderCallbackData eventPipeProviderCallbackData = pProvider->UnsetConfiguration(
-                        session.GetId(),
+                        session.GetMask(),
                         pSessionProvider->GetKeywords(),
                         pSessionProvider->GetLevel(),
                         pSessionProvider->GetFilterData());
diff --git a/src/vm/eventpipeconfiguration.h b/src/vm/eventpipeconfiguration.h
index f97b78f99a..a9148cb93c 100644
--- a/src/vm/eventpipeconfiguration.h
+++ b/src/vm/eventpipeconfiguration.h
@@ -49,59 +49,13 @@ public:
         const EventPipeSession &session,
         EventPipeProviderCallbackDataQueue *pEventPipeProviderCallbackDataQueue);
 
-    // Get the status of the event pipe.
-    bool Enabled() const
-    {
-        LIMITED_METHOD_CONTRACT;
-        return (m_activeSessions != 0);
-    }
-
     // Get the event used to write metadata to the event stream.
     EventPipeEventInstance *BuildEventMetadataEvent(EventPipeEventInstance &sourceInstance, unsigned int metdataId);
 
     // Delete deferred providers.
     void DeleteDeferredProviders();
 
-    // Create a new session.
-    EventPipeSession *CreateSession(
-        LPCWSTR strOutputPath,
-        IpcStream *const pStream,
-        EventPipeSessionType sessionType,
-        EventPipeSerializationFormat format,
-        unsigned int circularBufferSizeInMB,
-        const EventPipeProviderConfiguration *pProviders,
-        uint32_t numProviders,
-        bool rundownEnabled = false);
-
-    // Delete a session.
-    void DeleteSession(EventPipeSession *pSession);
-
-    // Check that a single bit is set.
-    static bool IsValidId(EventPipeSessionID id)
-    {
-        return (id > 0) && ((id & (id - 1)) == 0);
-    }
-
-    // Check that a session Id is enabled.
-    bool IsSessionIdValid(EventPipeSessionID id)
-    {
-        return IsValidId(id) && (m_activeSessions & id);
-    }
-
 private:
-    // Helper function used to locate a free index in the range 0 - EventPipe::MaxNumberOfSessions
-    // Returns EventPipe::MaxNumberOfSessions if there are no free indexes
-    unsigned int GenerateSessionIndex() const
-    {
-        LIMITED_METHOD_CONTRACT;
-
-        uint64_t id = 1;
-        for (unsigned int i = 0; i < 64; ++i, id <<= i)
-            if ((m_activeSessions & id) == 0)
-                return i;
-        return EventPipe::MaxNumberOfSessions;
-    }
-
     // Get the provider without taking the lock.
     EventPipeProvider *GetProviderNoLock(const SString &providerID);
 
@@ -120,9 +74,6 @@ private:
     // The provider name for the configuration event pipe provider.
     // This provider is used to emit configuration events.
     const static WCHAR *s_configurationProviderName;
-
-    // Bitmask tracking EventPipe active sessions.
-    uint64_t m_activeSessions = 0;
 };
 
 #endif // FEATURE_PERFTRACING
diff --git a/src/vm/eventpipeevent.cpp b/src/vm/eventpipeevent.cpp
index de8069833f..40ef588754 100644
--- a/src/vm/eventpipeevent.cpp
+++ b/src/vm/eventpipeevent.cpp
@@ -159,11 +159,11 @@ void EventPipeEvent::RefreshState()
     m_enabled = m_pProvider->EventEnabled(m_keywords, m_level);
 }
 
-bool EventPipeEvent::IsEnabled(uint64_t sessionId) const
+bool EventPipeEvent::IsEnabled(uint64_t sessionMask) const
 {
     LIMITED_METHOD_CONTRACT;
     _ASSERT(m_pProvider != nullptr);
-    const bool IsProviderEnabled = m_pProvider->IsEnabled(sessionId);
+    const bool IsProviderEnabled = m_pProvider->IsEnabled(sessionMask);
     const bool IsEventEnabled = m_pProvider->EventEnabled(m_keywords, m_level);
     return (IsProviderEnabled && IsEventEnabled);
 }
diff --git a/src/vm/eventpipeevent.h b/src/vm/eventpipeevent.h
index 75a767d543..122c3bef81 100644
--- a/src/vm/eventpipeevent.h
+++ b/src/vm/eventpipeevent.h
@@ -79,7 +79,7 @@ public:
 
     unsigned int GetMetadataLength() const;
 
-    bool IsEnabled(uint64_t sessionId) const;
+    bool IsEnabled(uint64_t sessionMask) const;
 
 private:
     // used when Metadata is not provided
diff --git a/src/vm/eventpipeprovider.cpp b/src/vm/eventpipeprovider.cpp
index 525571bc09..eb32dd8efc 100644
--- a/src/vm/eventpipeprovider.cpp
+++ b/src/vm/eventpipeprovider.cpp
@@ -107,7 +107,7 @@ bool EventPipeProvider::EventEnabled(INT64 keywords, EventPipeEventLevel eventLe
 }
 
 EventPipeProviderCallbackData EventPipeProvider::SetConfiguration(
-    uint64_t sessionId,
+    uint64_t sessionMask,
     INT64 keywords,
     EventPipeEventLevel providerLevel,
     LPCWSTR pFilterData)
@@ -117,12 +117,12 @@ EventPipeProviderCallbackData EventPipeProvider::SetConfiguration(
         THROWS;
         GC_TRIGGERS;
         MODE_ANY;
-        PRECONDITION(sessionId != 0);
+        PRECONDITION((m_sessions & sessionMask) == 0);
         PRECONDITION(EventPipe::IsLockOwnedByCurrentThread());
     }
     CONTRACTL_END;
 
-    m_sessions |= sessionId;
+    m_sessions |= sessionMask;
 
     // Set Keywords to be the union of all keywords
     m_keywords |= keywords;
@@ -130,12 +130,12 @@ EventPipeProviderCallbackData EventPipeProvider::SetConfiguration(
     // Set the provider level to "Log Always" or the biggest verbosity.
     m_providerLevel = (providerLevel < m_providerLevel) ? m_providerLevel : providerLevel;
 
-    RefreshAllEvents(sessionId, keywords, providerLevel);
+    RefreshAllEvents(sessionMask, keywords, providerLevel);
     return PrepareCallbackData(keywords, providerLevel, pFilterData);
 }
 
 EventPipeProviderCallbackData EventPipeProvider::UnsetConfiguration(
-        uint64_t sessionId,
+        uint64_t sessionMask,
         INT64 keywords,
         EventPipeEventLevel providerLevel,
         LPCWSTR pFilterData)
@@ -145,13 +145,13 @@ EventPipeProviderCallbackData EventPipeProvider::UnsetConfiguration(
         THROWS;
         GC_TRIGGERS;
         MODE_ANY;
-        PRECONDITION((m_sessions & sessionId) != 0);
+        PRECONDITION((m_sessions & sessionMask) != 0);
         PRECONDITION(EventPipe::IsLockOwnedByCurrentThread());
     }
     CONTRACTL_END;
 
-    if (m_sessions & sessionId)
-        m_sessions &= ~sessionId;
+    if (m_sessions & sessionMask)
+        m_sessions &= ~sessionMask;
     return PrepareCallbackData(keywords, providerLevel, pFilterData);
 }
 
@@ -294,7 +294,7 @@ void EventPipeProvider::SetDeleteDeferred()
 }
 
 void EventPipeProvider::RefreshAllEvents(
-    uint64_t sessionId,
+    uint64_t sessionMask,
     INT64 keywords,
     EventPipeEventLevel providerLevel)
 {
diff --git a/src/vm/eventpipeprovider.h b/src/vm/eventpipeprovider.h
index 4bfe1ce8b0..b8ae87edfb 100644
--- a/src/vm/eventpipeprovider.h
+++ b/src/vm/eventpipeprovider.h
@@ -66,10 +66,10 @@ public:
         return (m_sessions != 0);
     }
 
-    bool IsEnabled(uint64_t sessionId) const
+    bool IsEnabled(uint64_t sessionMask) const
     {
         LIMITED_METHOD_CONTRACT;
-        return ((m_sessions & sessionId) != 0);
+        return ((m_sessions & sessionMask) != 0);
     }
 
     // Determine if the specified keywords are enabled.
@@ -89,7 +89,7 @@ private:
     // Set the provider configuration (enable sets of events).
     // This is called by EventPipeConfiguration.
     EventPipeProviderCallbackData SetConfiguration(
-        uint64_t sessionId,
+        uint64_t sessionMask,
         INT64 keywords,
         EventPipeEventLevel providerLevel,
         LPCWSTR pFilterData);
@@ -97,14 +97,14 @@ private:
     // Unset the provider configuration for the specified session (disable sets of events).
     // This is called by EventPipeConfiguration.
     EventPipeProviderCallbackData UnsetConfiguration(
-        uint64_t sessionId,
+        uint64_t sessionMask,
         INT64 keywords,
         EventPipeEventLevel providerLevel,
         LPCWSTR pFilterData);
 
     // Refresh the runtime state of all events.
     void RefreshAllEvents(
-        uint64_t sessionId,
+        uint64_t sessionMask,
         INT64 keywords,
         EventPipeEventLevel providerLevel);
 
diff --git a/src/vm/eventpipesession.cpp b/src/vm/eventpipesession.cpp
index cb0140763c..e7d657bd9a 100644
--- a/src/vm/eventpipesession.cpp
+++ b/src/vm/eventpipesession.cpp
@@ -13,16 +13,15 @@
 #ifdef FEATURE_PERFTRACING
 
 EventPipeSession::EventPipeSession(
-    unsigned int index,
+    uint32_t index,
     LPCWSTR strOutputPath,
     IpcStream *const pStream,
     EventPipeSessionType sessionType,
     EventPipeSerializationFormat format,
-    unsigned int circularBufferSizeInMB,
+    uint32_t circularBufferSizeInMB,
     const EventPipeProviderConfiguration *pProviders,
     uint32_t numProviders,
-    bool rundownEnabled) : m_Id((EventPipeSessionID)1 << index),
-                           m_index(index),
+    bool rundownEnabled) : m_index(index),
                            m_pProviderList(new EventPipeSessionProviderList(pProviders, numProviders)),
                            m_rundownEnabled(rundownEnabled),
                            m_SessionType(sessionType),
@@ -64,7 +63,7 @@ EventPipeSession::EventPipeSession(
         break;
 
     case EventPipeSessionType::IpcStream:
-        m_pFile = new EventPipeFile(new IpcStreamWriter(m_Id, pStream), format);
+        m_pFile = new EventPipeFile(new IpcStreamWriter(reinterpret_cast<uint64_t>(this), pStream), format);
         break;
 
     default:
@@ -83,10 +82,10 @@ EventPipeSession::~EventPipeSession()
         NOTHROW;
         GC_TRIGGERS;
         MODE_PREEMPTIVE;
+        PRECONDITION(!m_ipcStreamingEnabled);
     }
     CONTRACTL_END;
 
-    // TODO: Stop streaming thread? Review synchronization.
     delete m_pProviderList;
     delete m_pBufferManager;
     delete m_pFile;
@@ -119,21 +118,6 @@ void EventPipeSession::SetThreadShutdownEvent()
     m_threadShutdownEvent.Set();
 }
 
-void EventPipeSession::DestroyIpcStreamingThread()
-{
-    CONTRACTL
-    {
-        NOTHROW;
-        GC_TRIGGERS;
-        MODE_COOPERATIVE;
-    }
-    CONTRACTL_END;
-
-    if (m_pIpcStreamingThread != nullptr)
-        ::DestroyThread(m_pIpcStreamingThread);
-    m_pIpcStreamingThread = nullptr;
-}
-
 static void PlatformSleep()
 {
     CONTRACTL
@@ -176,11 +160,13 @@ DWORD WINAPI EventPipeSession::ThreadProc(void *args)
     if (!pEventPipeSession->HasIpcStreamingStarted())
         return 1;
 
+    Thread *const pThisThread = pEventPipeSession->GetIpcStreamingThread();
+    bool fSuccess = true;
+
     {
         GCX_PREEMP();
         EX_TRY
         {
-            bool fSuccess = true;
             while (pEventPipeSession->IsIpcStreamingEnabled())
             {
                 if (!pEventPipeSession->WriteAllBuffersToFile())
@@ -194,14 +180,6 @@ DWORD WINAPI EventPipeSession::ThreadProc(void *args)
             }
 
             pEventPipeSession->SetThreadShutdownEvent();
-
-            if (!fSuccess)
-            {
-                // TODO: Notify `EventPipe::Disable` instead, this would disable the session, and remove it from the active list.
-                EventPipe::RunWithCallbackPostponed([pEventPipeSession](EventPipeProviderCallbackDataQueue *pEventPipeProviderCallbackDataQueue) {
-                    pEventPipeSession->Disable();
-                });
-            }
         }
         EX_CATCH
         {
@@ -212,7 +190,20 @@ DWORD WINAPI EventPipeSession::ThreadProc(void *args)
         EX_END_CATCH(SwallowAllExceptions);
     }
 
-    pEventPipeSession->DestroyIpcStreamingThread();
+    EX_TRY
+    {
+        if (!fSuccess)
+            EventPipe::Disable(reinterpret_cast<EventPipeSessionID>(pEventPipeSession));
+    }
+    EX_CATCH
+    {
+        // TODO: STRESS_LOG ?
+    }
+    EX_END_CATCH(SwallowAllExceptions);
+
+    if (pThisThread != nullptr)
+        ::DestroyThread(pThisThread);
+
     return 0;
 }
 
@@ -241,7 +232,7 @@ void EventPipeSession::CreateIpcStreamingThread()
     m_threadShutdownEvent.CreateManualEvent(FALSE);
 }
 
-bool EventPipeSession::IsValid()
+bool EventPipeSession::IsValid() const
 {
     CONTRACTL
     {
@@ -321,7 +312,7 @@ bool EventPipeSession::WriteEventBuffered(
     CONTRACTL_END;
 
     // Filter events specific to "this" session based on precomputed flag on provider/events.
-    return event.IsEnabled(GetId()) ?
+    return event.IsEnabled(GetMask()) ?
         m_pBufferManager->WriteEvent(pThread, *this, event, payload, pActivityId, pRelatedActivityId, pEventThread, pStack) :
         false;
 }
@@ -338,16 +329,13 @@ void EventPipeSession::WriteEventUnbuffered(EventPipeEventInstance &instance, Ev
 
     if (m_pFile == nullptr)
         return;
-    EventPipeThreadSessionState* pState = nullptr;
     ULONGLONG captureThreadId;
-    unsigned int sequenceNumber;
+    uint32_t sequenceNumber;
     {
         SpinLockHolder _slh(pThread->GetLock());
-        pState = pThread->GetSessionState(this);
+        EventPipeThreadSessionState *const pState = pThread->GetOrCreateSessionState(this);
         if (pState == nullptr)
-        {
             return;
-        }
         captureThreadId = pThread->GetOSThreadId();
         sequenceNumber = pState->GetSequenceNumber();
         pState->IncrementSequenceNumber();
@@ -455,21 +443,20 @@ void EventPipeSession::DisableIpcStreamingThread()
         THROWS;
         GC_TRIGGERS;
         MODE_PREEMPTIVE;
+        PRECONDITION(m_SessionType == EventPipeSessionType::IpcStream);
+        PRECONDITION(m_ipcStreamingEnabled);
     }
     CONTRACTL_END;
 
-    if ((m_SessionType == EventPipeSessionType::IpcStream) && m_ipcStreamingEnabled)
-    {
-        _ASSERTE(!g_fProcessDetach);
+    _ASSERTE(!g_fProcessDetach);
 
-        // The IPC streaming thread will watch this value and exit
-        // when profiling is disabled.
-        m_ipcStreamingEnabled = false;
+    // The IPC streaming thread will watch this value and exit
+    // when profiling is disabled.
+    m_ipcStreamingEnabled = false;
 
-        // Wait for the sampling thread to clean itself up.
-        m_threadShutdownEvent.Wait(INFINITE, FALSE /* bAlertable */);
-        m_threadShutdownEvent.CloseEvent();
-    }
+    // Wait for the sampling thread to clean itself up.
+    m_threadShutdownEvent.Wait(INFINITE, FALSE /* bAlertable */);
+    m_threadShutdownEvent.CloseEvent();
 }
 
 void EventPipeSession::Disable()
@@ -482,11 +469,12 @@ void EventPipeSession::Disable()
     }
     CONTRACTL_END;
 
-    DisableIpcStreamingThread();
+    if ((m_SessionType == EventPipeSessionType::IpcStream) && m_ipcStreamingEnabled)
+        DisableIpcStreamingThread();
 
     // Force all in-progress writes to either finish or cancel
     // This is required to ensure we can safely flush and delete the buffers
-    m_pBufferManager->SuspendWriteEvent(GetId());
+    m_pBufferManager->SuspendWriteEvent(GetIndex());
     WriteAllBuffersToFile();
     m_pProviderList->Clear();
 }
diff --git a/src/vm/eventpipesession.h b/src/vm/eventpipesession.h
index 73e6a60f15..9b2340389d 100644
--- a/src/vm/eventpipesession.h
+++ b/src/vm/eventpipesession.h
@@ -29,7 +29,7 @@ enum class EventPipeSessionType
 enum class EventPipeSerializationFormat
 {
     // Default format used in .Net Core 2.0-3.0 Preview 6
-    // TBD - it may remain the default format .Net Core 3.0 when 
+    // TBD - it may remain the default format .Net Core 3.0 when
     // used with private EventPipe managed API via reflection.
     // This format had limited official exposure in documented
     // end-user RTM scenarios, but it is supported by PerfView,
@@ -43,12 +43,11 @@ enum class EventPipeSerializationFormat
     Count
 };
 
+//! Encapsulates an EventPipe session information and memory management.
 class EventPipeSession
 {
 private:
-
-    const EventPipeSessionID m_Id;
-    const unsigned int m_index;
+    const uint32_t m_index;
 
     // The set of configurations for each provider in the session.
     EventPipeSessionProviderList *const m_pProviderList;
@@ -79,42 +78,40 @@ private:
     // Data members used when an IPC streaming thread is used.
     Volatile<BOOL> m_ipcStreamingEnabled = false;
 
-    //
+    // When the session is of IPC type, this becomes a handle to the streaming thread.
     Thread *m_pIpcStreamingThread = nullptr;
 
-    //
+    // Event object used to signal Disable that the IPC streaming thread is done.
     CLREvent m_threadShutdownEvent;
 
     void CreateIpcStreamingThread();
 
     static DWORD WINAPI ThreadProc(void *args);
 
-    void DestroyIpcStreamingThread();
-
     void SetThreadShutdownEvent();
 
     void DisableIpcStreamingThread();
 
 public:
     EventPipeSession(
-        unsigned int index,
+        uint32_t index,
         LPCWSTR strOutputPath,
         IpcStream *const pStream,
         EventPipeSessionType sessionType,
         EventPipeSerializationFormat format,
-        unsigned int circularBufferSizeInMB,
+        uint32_t circularBufferSizeInMB,
         const EventPipeProviderConfiguration *pProviders,
         uint32_t numProviders,
         bool rundownEnabled = false);
     ~EventPipeSession();
 
-    EventPipeSessionID GetId() const
+    uint64_t GetMask() const
     {
         LIMITED_METHOD_CONTRACT;
-        return m_Id;
+        return (1ui64 << m_index);
     }
 
-    unsigned int GetIndex() const
+    uint32_t GetIndex() const
     {
         LIMITED_METHOD_CONTRACT;
         return m_index;
@@ -169,6 +166,12 @@ public:
     }
 #endif
 
+    Thread *GetIpcStreamingThread() const
+    {
+        LIMITED_METHOD_CONTRACT;
+        return m_pIpcStreamingThread;
+    }
+
     // Add a new provider to the session.
     void AddSessionProvider(EventPipeSessionProvider *pProvider);
 
@@ -203,7 +206,7 @@ public:
     void ExecuteRundown();
 
     // Determine if the session is valid or not.  Invalid sessions can be detected before they are enabled.
-    bool IsValid() /* This is not const because CrtsHolder does not take a const* */;
+    bool IsValid() const;
 
     bool HasIpcStreamingStarted() /* This is not const because CrtsHolder does not take a const* */;
 };
diff --git a/src/vm/eventpipethread.cpp b/src/vm/eventpipethread.cpp
index 60091a6eeb..0849f0b7c1 100644
--- a/src/vm/eventpipethread.cpp
+++ b/src/vm/eventpipethread.cpp
@@ -188,33 +188,30 @@ void EventPipeThread::Release()
     }
 }
 
-EventPipeThreadSessionState* EventPipeThread::GetOrCreateSessionState(EventPipeSession* pSession)
+EventPipeThreadSessionState *EventPipeThread::GetOrCreateSessionState(EventPipeSession *pSession)
 {
     LIMITED_METHOD_CONTRACT;
-    _ASSERTE(pSession != nullptr);
-    _ASSERTE(IsLockOwnedByCurrentThread());
+    PRECONDITION(pSession != nullptr);
+    PRECONDITION(pSession->GetIndex() < EventPipe::MaxNumberOfSessions);
+    PRECONDITION(IsLockOwnedByCurrentThread());
 
-    unsigned int index = pSession->GetIndex();
-    _ASSERTE(index < EventPipe::MaxNumberOfSessions);
-    EventPipeThreadSessionState* pState = m_sessionState[index];
+    EventPipeThreadSessionState *pState = m_sessionState[pSession->GetIndex()];
     if (pState == nullptr)
     {
         pState = new (nothrow) EventPipeThreadSessionState(this, pSession DEBUG_ARG(pSession->GetBufferManager()));
-        m_sessionState[index] = pState;
+        m_sessionState[pSession->GetIndex()] = pState;
     }
     return pState;
 }
 
-EventPipeThreadSessionState* EventPipeThread::GetSessionState(EventPipeSession* pSession)
+EventPipeThreadSessionState *EventPipeThread::GetSessionState(EventPipeSession *pSession)
 {
     LIMITED_METHOD_CONTRACT;
-    _ASSERTE(pSession != nullptr);
-    _ASSERTE(IsLockOwnedByCurrentThread());
-
-    unsigned int index = pSession->GetIndex();
-    _ASSERTE(index < EventPipe::MaxNumberOfSessions);
-    EventPipeThreadSessionState* pState = m_sessionState[index];
+    PRECONDITION(pSession != nullptr);
+    PRECONDITION(pSession->GetIndex() < EventPipe::MaxNumberOfSessions);
+    PRECONDITION(IsLockOwnedByCurrentThread());
 
+    EventPipeThreadSessionState *const pState = m_sessionState[pSession->GetIndex()];
     _ASSERTE(pState != nullptr);
     return pState;
 }
diff --git a/src/vm/eventpipethread.h b/src/vm/eventpipethread.h
index 7f82a2097d..26b9a71fa2 100644
--- a/src/vm/eventpipethread.h
+++ b/src/vm/eventpipethread.h
@@ -29,7 +29,7 @@ class EventPipeThreadSessionState
     // immutable
     EventPipeSession* m_pSession;
 
-    // The buffer this thread is allowed to write to if non-null, it must 
+    // The buffer this thread is allowed to write to if non-null, it must
     // match the tail of m_bufferList
     // protected by m_pThread::GetLock()
     EventPipeBuffer* m_pWriteBuffer;
@@ -116,7 +116,7 @@ class EventPipeThread
     // If this is set to a valid id before the corresponding entry of s_pSessions is set to null,
     // that pointer will be protected from deletion. See EventPipe::DisableInternal() and
     // EventPipe::WriteInternal for more detail.
-    Volatile<EventPipeSessionID> m_writingEventInProgress;
+    Volatile<uint32_t> m_writingEventInProgress;
 
     //
     EventPipeSession *m_pRundownSession = nullptr;
@@ -156,13 +156,13 @@ public:
         return m_pRundownSession;
     }
 
-    void SetSessionWriteInProgress(uint64_t index)
+    void SetSessionWriteInProgress(uint32_t sessionIndex)
     {
         LIMITED_METHOD_CONTRACT;
-        m_writingEventInProgress.Store((index < 64) ? (1ULL << index) : UINT64_MAX);
+        m_writingEventInProgress.Store(sessionIndex);
     }
 
-    EventPipeSessionID GetSessionWriteInProgress() const
+    uint32_t GetSessionWriteInProgress() const
     {
         LIMITED_METHOD_CONTRACT;
         return m_writingEventInProgress.Load();