Delay Diagnostics IPC response till after EventPipe::Enable has completed (dotnet...

* Refactor IPC response to after EventPipe::Enable is finished
* Refactor EventPipeFile initialization and EventPipeBufferManager flushing to be manually started after the IPC response has been sent
* Change EventPipeSession::Enable to EventPipeSession::StartStreaming

Commit migrated from https://github.com/dotnet/coreclr/commit/1898c0f4e99a476bb6d14f7b05f06560c349465f

diff --git a/src/coreclr/src/vm/eventpipe.cpp b/src/coreclr/src/vm/eventpipe.cpp
index 7eab2c8..16d2db5 100644 (file)
--- a/src/coreclr/src/vm/eventpipe.cpp
+++ b/src/coreclr/src/vm/eventpipe.cpp
@@ -269,12 +269,29 @@ bool EventPipe::EnableInternal(
     // Enable the sample profiler
     SampleProfiler::Enable(pEventPipeProviderCallbackDataQueue);
 
-    // Enable the session.
-    pSession->Enable();
-
     return true;
 }
 
+void EventPipe::StartStreaming(EventPipeSessionID id)
+{
+    CONTRACTL
+    {
+        THROWS;
+        GC_TRIGGERS;
+        MODE_ANY;
+    }
+    CONTRACTL_END;
+
+    CrstHolder _crst(GetLock());
+
+    if (!IsSessionIdInCollection(id))
+        return;
+
+    EventPipeSession *const pSession = reinterpret_cast<EventPipeSession *>(id);
+
+    pSession->StartStreaming();
+}
+
 void EventPipe::Disable(EventPipeSessionID id)
 {
     CONTRACTL

diff --git a/src/coreclr/src/vm/eventpipe.h b/src/coreclr/src/vm/eventpipe.h
index ac234a0..f6a60a9 100644 (file)
--- a/src/coreclr/src/vm/eventpipe.h
+++ b/src/coreclr/src/vm/eventpipe.h
@@ -67,6 +67,10 @@ public:
     // Get the session for the specified session ID.
     static EventPipeSession *GetSession(EventPipeSessionID id);
 
+    // start sending the required events down the pipe
+    // starting with file header info and then buffered events
+    static void StartStreaming(EventPipeSessionID id);
+
     // Specifies whether or not the event pipe is enabled.
     static bool Enabled()
     {

diff --git a/src/coreclr/src/vm/eventpipefile.cpp b/src/coreclr/src/vm/eventpipefile.cpp
index 621a4cd..bcaf266 100644 (file)
--- a/src/coreclr/src/vm/eventpipefile.cpp
+++ b/src/coreclr/src/vm/eventpipefile.cpp
@@ -77,7 +77,9 @@ DWORD GetFileMinVersion(EventPipeSerializationFormat format)
 }
 
 EventPipeFile::EventPipeFile(StreamWriter *pStreamWriter, EventPipeSerializationFormat format) :
-    FastSerializableObject(GetFileVersion(format), GetFileMinVersion(format), format >= EventPipeSerializationFormat::NetTraceV4)
+    FastSerializableObject(GetFileVersion(format), GetFileMinVersion(format), format >= EventPipeSerializationFormat::NetTraceV4),
+    m_pSerializer(nullptr),
+    m_pStreamWriter(pStreamWriter)
 {
     CONTRACTL
     {
@@ -107,22 +109,7 @@ EventPipeFile::EventPipeFile(StreamWriter *pStreamWriter, EventPipeSerialization
 
     m_samplingRateInNs = SampleProfiler::GetSamplingRate();
 
-    bool fSuccess = true;
-    if (m_format >= EventPipeSerializationFormat::NetTraceV4)
-    {
-        const char* pHeader = "Nettrace";
-        uint32_t bytesWritten = 0;
-        fSuccess = pStreamWriter->Write(pHeader, 8, bytesWritten) && bytesWritten == 8;
-    }
-    if (fSuccess)
-    {
-        // Create the file stream and write the FastSerialization header.
-        m_pSerializer = new FastSerializer(pStreamWriter);
-    }
-    else
-    {
-        m_pSerializer = nullptr;
-    }
+
 
     m_serializationLock.Init(LOCK_TYPE_DEFAULT);
 
@@ -138,8 +125,36 @@ EventPipeFile::EventPipeFile(StreamWriter *pStreamWriter, EventPipeSerialization
     QueryPerformanceCounter(&m_lastSortedTimestamp);
 #endif
 
-    // Write the first object to the file.
-    m_pSerializer->WriteObject(this);
+
+}
+
+void EventPipeFile::InitializeFile()
+{
+    CONTRACTL
+    {
+        THROWS;
+        GC_TRIGGERS;
+        MODE_PREEMPTIVE;
+        PRECONDITION(m_pStreamWriter != nullptr);
+        PRECONDITION(m_pSerializer == nullptr);
+    }
+    CONTRACTL_END;
+
+    bool fSuccess = true;
+    if (m_format >= EventPipeSerializationFormat::NetTraceV4)
+    {
+        const char* pHeader = "Nettrace";
+        uint32_t bytesWritten = 0;
+        fSuccess = m_pStreamWriter->Write(pHeader, 8, bytesWritten) && bytesWritten == 8;
+    }
+    if (fSuccess)
+    {
+        // Create the file stream and write the FastSerialization header.
+        m_pSerializer = new FastSerializer(m_pStreamWriter);
+        
+        // Write the first object to the file.
+        m_pSerializer->WriteObject(this);
+    }
 }
 
 EventPipeFile::~EventPipeFile()
@@ -186,6 +201,7 @@ void EventPipeFile::WriteEvent(EventPipeEventInstance &instance, ULONGLONG captu
         THROWS;
         GC_NOTRIGGER;
         MODE_ANY;
+        PRECONDITION(!HasErrors());
     }
     CONTRACTL_END;
 
@@ -231,6 +247,7 @@ void EventPipeFile::WriteSequencePoint(EventPipeSequencePoint* pSequencePoint)
         GC_NOTRIGGER;
         MODE_ANY;
         PRECONDITION(pSequencePoint != nullptr);
+        PRECONDITION(!HasErrors());
     }
     CONTRACTL_END;
 
@@ -261,8 +278,13 @@ void EventPipeFile::Flush(FlushFlags flags)
         NOTHROW;
         GC_NOTRIGGER;
         MODE_ANY;
+        PRECONDITION(!HasErrors());
+        PRECONDITION(m_pMetadataBlock != nullptr);
+        PRECONDITION(m_pStackBlock != nullptr);
+        PRECONDITION(m_pBlock != nullptr);
     }
     CONTRACTL_END;
+
     // we write current blocks to the disk, whether they are full or not
     if ((m_pMetadataBlock->GetBytesWritten() != 0) && ((flags & FlushMetadataBlock) != 0))
     {
@@ -290,6 +312,7 @@ void EventPipeFile::WriteEnd()
         NOTHROW;
         GC_NOTRIGGER;
         MODE_ANY;
+        PRECONDITION(!HasErrors());
     }
     CONTRACTL_END;
 
@@ -312,6 +335,8 @@ void EventPipeFile::WriteEventToBlock(EventPipeEventInstance &instance,
         THROWS;
         GC_NOTRIGGER;
         MODE_ANY;
+        PRECONDITION(m_pBlock != nullptr);
+        PRECONDITION(m_pMetadataBlock != nullptr);
     }
     CONTRACTL_END;
 
@@ -363,6 +388,7 @@ unsigned int EventPipeFile::GetMetadataId(EventPipeEvent &event)
         NOTHROW;
         GC_NOTRIGGER;
         MODE_ANY;
+        PRECONDITION(m_pMetadataIds != nullptr);
     }
     CONTRACTL_END;
 
@@ -384,6 +410,7 @@ void EventPipeFile::SaveMetadataId(EventPipeEvent &event, unsigned int metadataI
         GC_NOTRIGGER;
         MODE_ANY;
         PRECONDITION(metadataId > 0);
+        PRECONDITION(m_pMetadataIds != nullptr);
     }
     CONTRACTL_END;
 
@@ -404,6 +431,7 @@ unsigned int EventPipeFile::GetStackId(EventPipeEventInstance &instance)
         GC_NOTRIGGER;
         MODE_ANY;
         PRECONDITION(m_format >= EventPipeSerializationFormat::NetTraceV4);
+        PRECONDITION(m_pStackBlock != nullptr);
     }
     CONTRACTL_END;
 

diff --git a/src/coreclr/src/vm/eventpipefile.h b/src/coreclr/src/vm/eventpipefile.h
index 2cc17ac..68873b6 100644 (file)
--- a/src/coreclr/src/vm/eventpipefile.h
+++ b/src/coreclr/src/vm/eventpipefile.h
@@ -75,6 +75,7 @@ public:
     EventPipeFile(StreamWriter *pStreamWriter, EventPipeSerializationFormat format);
     ~EventPipeFile();
 
+    void InitializeFile();
     EventPipeSerializationFormat GetSerializationFormat() const;
     void WriteEvent(EventPipeEventInstance &instance, ULONGLONG captureThreadId, unsigned int sequenceNumber, BOOL isSortedEvent);
     void WriteSequencePoint(EventPipeSequencePoint* pSequencePoint);
@@ -153,6 +154,8 @@ private:
     // The frequency of the timestamps used for this file.
     LARGE_INTEGER m_timeStampFrequency;
 
+    StreamWriter * const m_pStreamWriter;
+
     unsigned int m_pointerSize;
 
     unsigned int m_currentProcessId;

diff --git a/src/coreclr/src/vm/eventpipeinternal.cpp b/src/coreclr/src/vm/eventpipeinternal.cpp
index c99c103..17c383d 100644 (file)
--- a/src/coreclr/src/vm/eventpipeinternal.cpp
+++ b/src/coreclr/src/vm/eventpipeinternal.cpp
@@ -48,6 +48,7 @@ UINT64 QCALLTYPE EventPipeInternal::Enable(
             format,
             true,
             nullptr);
+        EventPipe::StartStreaming(sessionID);
     }
     END_QCALL;
 

diff --git a/src/coreclr/src/vm/eventpipeprotocolhelper.cpp b/src/coreclr/src/vm/eventpipeprotocolhelper.cpp
index e60e998..7cac9d0 100644 (file)
--- a/src/coreclr/src/vm/eventpipeprotocolhelper.cpp
+++ b/src/coreclr/src/vm/eventpipeprotocolhelper.cpp
@@ -224,11 +224,10 @@ void EventPipeProtocolHelper::CollectTracing(DiagnosticsIpc::IpcMessage& message
     }
     CONTRACTL_END;
 
-    const EventPipeCollectTracingCommandPayload* payload = message.TryParsePayload<EventPipeCollectTracingCommandPayload>();
+    NewHolder<const EventPipeCollectTracingCommandPayload> payload = message.TryParsePayload<EventPipeCollectTracingCommandPayload>();
     if (payload == nullptr)
     {
         DiagnosticsIpc::IpcMessage::SendErrorMessage(pStream, CORDIAGIPC_E_BAD_ENCODING);
-        delete payload;
         delete pStream;
         return;
     }
@@ -246,9 +245,15 @@ void EventPipeProtocolHelper::CollectTracing(DiagnosticsIpc::IpcMessage& message
     if (sessionId == 0)
     {
         DiagnosticsIpc::IpcMessage::SendErrorMessage(pStream, E_FAIL);
-        delete payload;
         delete pStream;
     }
+    else
+    {
+        DiagnosticsIpc::IpcMessage successResponse;
+        if (successResponse.Initialize(DiagnosticsIpc::GenericSuccessHeader, sessionId))
+            successResponse.Send(pStream);
+        EventPipe::StartStreaming(sessionId);
+    }
 }
 
 void EventPipeProtocolHelper::CollectTracing2(DiagnosticsIpc::IpcMessage& message, IpcStream *pStream)

diff --git a/src/coreclr/src/vm/eventpipesession.cpp b/src/coreclr/src/vm/eventpipesession.cpp
index 8d2ff65..6068c44 100644 (file)
--- a/src/coreclr/src/vm/eventpipesession.cpp
+++ b/src/coreclr/src/vm/eventpipesession.cpp
@@ -365,7 +365,7 @@ CLREvent *EventPipeSession::GetWaitEvent()
     return m_pBufferManager->GetWaitEvent();
 }
 
-void EventPipeSession::Enable()
+void EventPipeSession::StartStreaming()
 {
     CONTRACTL
     {
@@ -377,6 +377,8 @@ void EventPipeSession::Enable()
     }
     CONTRACTL_END;
 
+    m_pFile->InitializeFile();
+
     if (m_SessionType == EventPipeSessionType::IpcStream)
         CreateIpcStreamingThread();
 }

diff --git a/src/coreclr/src/vm/eventpipesession.h b/src/coreclr/src/vm/eventpipesession.h
index 47d4040..adcddb3 100644 (file)
--- a/src/coreclr/src/vm/eventpipesession.h
+++ b/src/coreclr/src/vm/eventpipesession.h
@@ -208,7 +208,11 @@ public:
     CLREvent *GetWaitEvent();
 
     // Enable a session in the event pipe.
-    void Enable();
+    // MUST be called AFTER sending the IPC response
+    // Side effects:
+    // - sends file header information for nettrace format
+    // - turns on IpcStreaming thread which flushes events to stream
+    void StartStreaming();
 
     // Disable a session in the event pipe.
     // side-effects: writes all buffers to stream/file

diff --git a/src/coreclr/src/vm/fastserializer.cpp b/src/coreclr/src/vm/fastserializer.cpp
index ccef926..1a529a4 100644 (file)
--- a/src/coreclr/src/vm/fastserializer.cpp
+++ b/src/coreclr/src/vm/fastserializer.cpp
@@ -27,10 +27,6 @@ IpcStreamWriter::IpcStreamWriter(uint64_t id, IpcStream *pStream) : _pStream(pSt
 
     if (_pStream == nullptr)
         return;
-
-    DiagnosticsIpc::IpcMessage successResponse;
-    if (successResponse.Initialize(DiagnosticsIpc::GenericSuccessHeader, id))
-        successResponse.Send(pStream);
 }
 
 IpcStreamWriter::~IpcStreamWriter()