Rebrand Diagnostics Server to DiagnosticPort

src/coreclr/src/debug/debug-pal/unix/diagnosticsipc.cpp

@@ -61,8 +61,8 @@ IpcStream::DiagnosticsIpc *IpcStream::DiagnosticsIpc::Create(const char *const p
             "socket");
     }
 
-    if (mode == ConnectionMode::CLIENT)
-        return new IpcStream::DiagnosticsIpc(-1, &serverAddress, ConnectionMode::CLIENT);
+    if (mode == ConnectionMode::CONNECT)
+        return new IpcStream::DiagnosticsIpc(-1, &serverAddress, ConnectionMode::CONNECT);
 
 #ifdef __APPLE__
     mode_t prev_mask = umask(~(S_IRUSR | S_IWUSR)); // This will set the default permission bit to 600
@@ -116,8 +116,8 @@ IpcStream::DiagnosticsIpc *IpcStream::DiagnosticsIpc::Create(const char *const p
 
 bool IpcStream::DiagnosticsIpc::Listen(ErrorCallback callback)
 {
-    _ASSERTE(mode == ConnectionMode::SERVER);
-    if (mode != ConnectionMode::SERVER)
+    _ASSERTE(mode == ConnectionMode::LISTEN);
+    if (mode != ConnectionMode::LISTEN)
     {
         if (callback != nullptr)
             callback("Cannot call Listen on a client connection", -1);
@@ -150,7 +150,7 @@ bool IpcStream::DiagnosticsIpc::Listen(ErrorCallback callback)
 
 IpcStream *IpcStream::DiagnosticsIpc::Accept(ErrorCallback callback)
 {
-    _ASSERTE(mode == ConnectionMode::SERVER);
+    _ASSERTE(mode == ConnectionMode::LISTEN);
     _ASSERTE(_isListening);
 
     sockaddr_un from;
@@ -168,7 +168,7 @@ IpcStream *IpcStream::DiagnosticsIpc::Accept(ErrorCallback callback)
 
 IpcStream *IpcStream::DiagnosticsIpc::Connect(ErrorCallback callback)
 {
-    _ASSERTE(mode == ConnectionMode::CLIENT);
+    _ASSERTE(mode == ConnectionMode::CONNECT);
 
     sockaddr_un clientAddress{};
     clientAddress.sun_family = AF_UNIX;
@@ -194,7 +194,7 @@ IpcStream *IpcStream::DiagnosticsIpc::Connect(ErrorCallback callback)
         return nullptr;
     }
 
-    return new IpcStream(clientSocket, ConnectionMode::CLIENT);
+    return new IpcStream(clientSocket, ConnectionMode::CONNECT);
 }
 
 int32_t IpcStream::DiagnosticsIpc::Poll(IpcPollHandle *rgIpcPollHandles, uint32_t nHandles, int32_t timeoutMs, ErrorCallback callback)
@@ -208,7 +208,7 @@ int32_t IpcStream::DiagnosticsIpc::Poll(IpcPollHandle *rgIpcPollHandles, uint32_
         if (rgIpcPollHandles[i].pIpc != nullptr)
         {
             // SERVER
-            _ASSERTE(rgIpcPollHandles[i].pIpc->mode == ConnectionMode::SERVER);
+            _ASSERTE(rgIpcPollHandles[i].pIpc->mode == ConnectionMode::LISTEN);
             fd = rgIpcPollHandles[i].pIpc->_serverSocket;
         }
         else

src/coreclr/src/debug/debug-pal/win/diagnosticsipc.cpp

@@ -56,8 +56,8 @@ IpcStream::DiagnosticsIpc *IpcStream::DiagnosticsIpc::Create(const char *const p
 
 bool IpcStream::DiagnosticsIpc::Listen(ErrorCallback callback)
 {
-    _ASSERTE(mode == ConnectionMode::SERVER);
-    if (mode != ConnectionMode::SERVER)
+    _ASSERTE(mode == ConnectionMode::LISTEN);
+    if (mode != ConnectionMode::LISTEN)
     {
         if (callback != nullptr)
             callback("Cannot call Listen on a client connection", -1);
@@ -131,7 +131,7 @@ bool IpcStream::DiagnosticsIpc::Listen(ErrorCallback callback)
 IpcStream *IpcStream::DiagnosticsIpc::Accept(ErrorCallback callback)
 {
     _ASSERTE(_isListening);
-    _ASSERTE(mode == ConnectionMode::SERVER);
+    _ASSERTE(mode == ConnectionMode::LISTEN);
 
     DWORD dwDummy = 0;
     bool fSuccess = GetOverlappedResult(
@@ -148,7 +148,7 @@ IpcStream *IpcStream::DiagnosticsIpc::Accept(ErrorCallback callback)
     }
 
     // create new IpcStream using handle and reset the Server object so it can listen again
-    IpcStream *pStream = new IpcStream(_hPipe, ConnectionMode::SERVER);
+    IpcStream *pStream = new IpcStream(_hPipe, ConnectionMode::LISTEN);
 
     // reset the server
     _hPipe = INVALID_HANDLE_VALUE;
@@ -167,8 +167,8 @@ IpcStream *IpcStream::DiagnosticsIpc::Accept(ErrorCallback callback)
 
 IpcStream *IpcStream::DiagnosticsIpc::Connect(ErrorCallback callback)
 {
-    _ASSERTE(mode == ConnectionMode::CLIENT);
-    if (mode != ConnectionMode::CLIENT)
+    _ASSERTE(mode == ConnectionMode::CONNECT);
+    if (mode != ConnectionMode::CONNECT)
     {
         if (callback != nullptr)
             callback("Cannot call connect on a server connection", 0);
@@ -206,7 +206,7 @@ void IpcStream::DiagnosticsIpc::Close(bool isShutdown, ErrorCallback callback)
 
     if (_hPipe != INVALID_HANDLE_VALUE)
     {
-        if (mode == DiagnosticsIpc::ConnectionMode::SERVER)
+        if (mode == DiagnosticsIpc::ConnectionMode::LISTEN)
         {
             const BOOL fSuccessDisconnectNamedPipe = ::DisconnectNamedPipe(_hPipe);
             _ASSERTE(fSuccessDisconnectNamedPipe != 0);
@@ -248,7 +248,7 @@ void IpcStream::Close(ErrorCallback callback)
     {
         Flush();
 
-        if (_mode == DiagnosticsIpc::ConnectionMode::SERVER)
+        if (_mode == DiagnosticsIpc::ConnectionMode::LISTEN)
         {
             const BOOL fSuccessDisconnectNamedPipe = ::DisconnectNamedPipe(_hPipe);
             _ASSERTE(fSuccessDisconnectNamedPipe != 0);
@@ -281,7 +281,7 @@ int32_t IpcStream::DiagnosticsIpc::Poll(IpcPollHandle *rgIpcPollHandles, uint32_
         if (rgIpcPollHandles[i].pIpc != nullptr)
         {
             // SERVER
-            _ASSERTE(rgIpcPollHandles[i].pIpc->mode == DiagnosticsIpc::ConnectionMode::SERVER);
+            _ASSERTE(rgIpcPollHandles[i].pIpc->mode == DiagnosticsIpc::ConnectionMode::LISTEN);
             pHandles[i] = rgIpcPollHandles[i].pIpc->_oOverlap.hEvent;
         }
         else

src/coreclr/src/debug/inc/diagnosticsipc.h

@@ -31,8 +31,8 @@ class IpcStream final
     public:
         enum ConnectionMode
         {
-            CLIENT,
-            SERVER
+            CONNECT,
+            LISTEN
         };
 
         enum class PollEvents : uint8_t
@@ -102,7 +102,7 @@ class IpcStream final
         sockaddr_un *const _pServerAddress;
         bool _isClosed;
 
-        DiagnosticsIpc(const int serverSocket, sockaddr_un *const pServerAddress, ConnectionMode mode = ConnectionMode::SERVER);
+        DiagnosticsIpc(const int serverSocket, sockaddr_un *const pServerAddress, ConnectionMode mode = ConnectionMode::LISTEN);
 
         // Used to unlink the socket so it can be removed from the filesystem
         // when the last reference to it is closed.
@@ -113,7 +113,7 @@ class IpcStream final
         HANDLE _hPipe = INVALID_HANDLE_VALUE;
         OVERLAPPED _oOverlap = {};
 
-        DiagnosticsIpc(const char(&namedPipeName)[MaxNamedPipeNameLength], ConnectionMode mode = ConnectionMode::SERVER);
+        DiagnosticsIpc(const char(&namedPipeName)[MaxNamedPipeNameLength], ConnectionMode mode = ConnectionMode::LISTEN);
 #endif /* TARGET_UNIX */
 
         bool _isListening;
@@ -128,13 +128,13 @@ class IpcStream final
 private:
 #ifdef TARGET_UNIX
     int _clientSocket = -1;
-    IpcStream(int clientSocket, DiagnosticsIpc::ConnectionMode mode = DiagnosticsIpc::ConnectionMode::SERVER)
+    IpcStream(int clientSocket, DiagnosticsIpc::ConnectionMode mode = DiagnosticsIpc::ConnectionMode::LISTEN)
         : _clientSocket(clientSocket), _mode(mode) {}
 #else
     HANDLE _hPipe = INVALID_HANDLE_VALUE;
     OVERLAPPED _oOverlap = {};
     BOOL _isTestReading = false; // used to check whether we are already doing a 0-byte read to test for data
-    IpcStream(HANDLE hPipe, DiagnosticsIpc::ConnectionMode mode = DiagnosticsIpc::ConnectionMode::SERVER);
+    IpcStream(HANDLE hPipe, DiagnosticsIpc::ConnectionMode mode = DiagnosticsIpc::ConnectionMode::LISTEN);
 #endif /* TARGET_UNIX */
 
     DiagnosticsIpc::ConnectionMode _mode;

src/coreclr/src/inc/clrconfigvalues.h

@@ -710,10 +710,10 @@ RETAIL_CONFIG_DWORD_INFO(INTERNAL_EventPipeCircularMB, W("EventPipeCircularMB"),
 RETAIL_CONFIG_DWORD_INFO(INTERNAL_EventPipeProcNumbers, W("EventPipeProcNumbers"), 0, "Enable/disable capturing processor numbers in EventPipe event headers")
 
 //
-// Diagnostics Server
+// Diagnostics Ports
 //
-RETAIL_CONFIG_STRING_INFO_EX(EXTERNAL_DOTNET_DiagnosticsMonitorAddress, W("DOTNET_DiagnosticsMonitorAddress"), "NamedPipe path without '\\\\.\\pipe\\' on Windows; Full path of Unix Domain Socket on Linux/Unix.  Used for Diagnostics Monitoring Agents.", CLRConfig::DontPrependCOMPlus_);
-RETAIL_CONFIG_DWORD_INFO_EX(EXTERNAL_DOTNET_DiagnosticsMonitorPauseOnStart, W("DOTNET_DiagnosticsMonitorPauseOnStart"), 1, "If DOTNET_DiagnosticsMonitorAddress is set, this will cause the runtime to pause during startup.  Resume using the Diagnostics IPC ResumeStartup command.", CLRConfig::DontPrependCOMPlus_);
+RETAIL_CONFIG_DWORD_INFO_EX(EXTERNAL_DOTNET_DefaultDiagnosticPortSuspend, W("DOTNET_DefaultDiagnosticPortSuspend"), 0, "This sets the deafult diagnostic port to suspend causing the runtime to pause during startup before major subsystems are started.  Resume using the Diagnostics IPC ResumeStartup command on the default diagnostic port.", CLRConfig::DontPrependCOMPlus_);
+RETAIL_CONFIG_STRING_INFO_EX(EXTERNAL_DOTNET_DiagnosticPorts, W("DOTNET_DiagnosticPorts"), "A semicolon delimited list of additional Diagnostic Ports, where a Diagnostic Port is a NamedPipe path without '\\\\.\\pipe\\' on Windows or the full path of Unix Domain Socket on Linux/Unix followed by optional tags, e.g., '<path>,connect,nosuspend;<path>'", CLRConfig::DontPrependCOMPlus_);
 
 //
 // LTTng

src/coreclr/src/vm/diagnosticserver.cpp

@@ -22,6 +22,7 @@
 
 Volatile<bool> DiagnosticServer::s_shuttingDown(false);
 CLREventStatic *DiagnosticServer::s_ResumeRuntimeStartupEvent = nullptr;
+GUID DiagnosticsIpc::AdvertiseCookie_V1 = GUID_NULL;
 
 DWORD WINAPI DiagnosticServer::DiagnosticsServerThread(LPVOID)
 {
@@ -32,11 +33,11 @@ DWORD WINAPI DiagnosticServer::DiagnosticsServerThread(LPVOID)
 #endif
         GC_TRIGGERS;
         MODE_PREEMPTIVE;
-        PRECONDITION(s_shuttingDown || IpcStreamFactory::HasActiveConnections());
+        PRECONDITION(s_shuttingDown || IpcStreamFactory::HasActivePorts());
     }
     CONTRACTL_END;
 
-    if (!IpcStreamFactory::HasActiveConnections())
+    if (!IpcStreamFactory::HasActivePorts())
     {
         STRESS_LOG0(LF_DIAGNOSTICS_PORT, LL_ERROR, "Diagnostics IPC listener was undefined\n");
         return 1;
@@ -144,30 +145,21 @@ bool DiagnosticServer::Initialize()
                 szMessage);                                           // data2
         };
 
-        NewArrayHolder<char> address = nullptr;
-        CLRConfigStringHolder wAddress = CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_DOTNET_DiagnosticsMonitorAddress);
-        int nCharactersWritten = 0;
-        if (wAddress != nullptr)
+        // Initialize the RuntimeIndentifier before use
+        CoCreateGuid(&DiagnosticsIpc::AdvertiseCookie_V1);
+
+        // Ports can fail to be configured 
+        bool fAnyErrors = IpcStreamFactory::Configure(ErrorCallback);
+        if (fAnyErrors)
+            STRESS_LOG0(LF_DIAGNOSTICS_PORT, LL_ERROR, "At least one Diagnostic Port failed to be configured.\n");
+
+        if (IpcStreamFactory::AnySuspendedPorts())
         {
-            // By default, opts in to Pause on Start
             s_ResumeRuntimeStartupEvent = new CLREventStatic();
             s_ResumeRuntimeStartupEvent->CreateManualEvent(false);
-
-            nCharactersWritten = WideCharToMultiByte(CP_UTF8, 0, wAddress, -1, NULL, 0, NULL, NULL);
-            if (nCharactersWritten != 0)
-            {
-                address = new char[nCharactersWritten];
-                nCharactersWritten = WideCharToMultiByte(CP_UTF8, 0, wAddress, -1, address, nCharactersWritten, NULL, NULL);
-                assert(nCharactersWritten != 0);
-            }
-
-            // Create the client mode connection
-            fSuccess &= IpcStreamFactory::CreateClient(address, ErrorCallback);
         }
 
-        fSuccess &= IpcStreamFactory::CreateServer(nullptr, ErrorCallback);
-
-        if (IpcStreamFactory::HasActiveConnections())
+        if (IpcStreamFactory::HasActivePorts())
         {
 #ifdef FEATURE_AUTO_TRACE
             auto_trace_init();
@@ -184,7 +176,7 @@ bool DiagnosticServer::Initialize()
 
             if (hServerThread == NULL)
             {
-                IpcStreamFactory::CloseConnections();
+                IpcStreamFactory::ClosePorts();
 
                 // Failed to create IPC thread.
                 STRESS_LOG1(
@@ -229,7 +221,7 @@ bool DiagnosticServer::Shutdown()
 
     EX_TRY
     {
-        if (IpcStreamFactory::HasActiveConnections())
+        if (IpcStreamFactory::HasActivePorts())
         {
             auto ErrorCallback = [](const char *szMessage, uint32_t code) {
                 STRESS_LOG2(
@@ -266,22 +258,22 @@ void DiagnosticServer::PauseForDiagnosticsMonitor()
     }
     CONTRACTL_END;
 
-    CLRConfigStringHolder pDotnetDiagnosticsMonitorAddress = CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_DOTNET_DiagnosticsMonitorAddress);
-    if (pDotnetDiagnosticsMonitorAddress != nullptr)
+    if (IpcStreamFactory::AnySuspendedPorts())
     {
-        DWORD dwDotnetDiagnosticsMonitorPauseOnStart = CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_DOTNET_DiagnosticsMonitorPauseOnStart);
-        if (dwDotnetDiagnosticsMonitorPauseOnStart != 0)
+        _ASSERTE(s_ResumeRuntimeStartupEvent != nullptr && s_ResumeRuntimeStartupEvent->IsValid());
+        STRESS_LOG0(LF_DIAGNOSTICS_PORT, LL_ALWAYS, "The runtime has been configured to pause during startup and is awaiting a Diagnostics IPC ResumeStartup command.");
+        const DWORD dwFiveSecondWait = s_ResumeRuntimeStartupEvent->Wait(5000, false);
+        if (dwFiveSecondWait == WAIT_TIMEOUT)
         {
-            _ASSERTE(s_ResumeRuntimeStartupEvent != nullptr && s_ResumeRuntimeStartupEvent->IsValid());
-            STRESS_LOG0(LF_DIAGNOSTICS_PORT, LL_ALWAYS, "The runtime has been configured to pause during startup and is awaiting a Diagnostics IPC ResumeStartup command.");
-            const DWORD dwFiveSecondWait = s_ResumeRuntimeStartupEvent->Wait(5000, false);
-            if (dwFiveSecondWait == WAIT_TIMEOUT)
-            {
-                wprintf(W("The runtime has been configured to pause during startup and is awaiting a Diagnostics IPC ResumeStartup command from a server at '%s'.\n"), (LPWSTR)pDotnetDiagnosticsMonitorAddress);
-                fflush(stdout);
-                STRESS_LOG0(LF_DIAGNOSTICS_PORT, LL_ALWAYS, "The runtime has been configured to pause during startup and is awaiting a Diagnostics IPC ResumeStartup command and has waited 5 seconds.");
-                const DWORD dwWait = s_ResumeRuntimeStartupEvent->Wait(INFINITE, false);
-            }
+            CLRConfigStringHolder dotnetDiagnosticPortString = CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_DOTNET_DiagnosticPorts);
+            WCHAR empty[] = W("");
+            DWORD dotnetDiagnosticPortSuspend = CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_DOTNET_DefaultDiagnosticPortSuspend);
+            wprintf(W("The runtime has been configured to pause during startup and is awaiting a Diagnostics IPC ResumeStartup command from a Diagnostic Port.\n"));
+            wprintf(W("DOTNET_DiagnosticPorts=\"%s\"\n"), dotnetDiagnosticPortString == nullptr ? empty : dotnetDiagnosticPortString.GetValue());
+            wprintf(W("DOTNET_DefaultDiagnosticPortSuspend=%d\n"), dotnetDiagnosticPortSuspend);
+            fflush(stdout);
+            STRESS_LOG0(LF_DIAGNOSTICS_PORT, LL_ALWAYS, "The runtime has been configured to pause during startup and is awaiting a Diagnostics IPC ResumeStartup command and has waited 5 seconds.");
+            const DWORD dwWait = s_ResumeRuntimeStartupEvent->Wait(INFINITE, false);
         }
     }
     // allow wait failures to fall through and the runtime to continue coming up
@@ -290,7 +282,8 @@ void DiagnosticServer::PauseForDiagnosticsMonitor()
 void DiagnosticServer::ResumeRuntimeStartup()
 {
     LIMITED_METHOD_CONTRACT;
-    if (s_ResumeRuntimeStartupEvent != nullptr && s_ResumeRuntimeStartupEvent->IsValid())
+    IpcStreamFactory::ResumeCurrentPort();
+    if (!IpcStreamFactory::AnySuspendedPorts() && s_ResumeRuntimeStartupEvent != nullptr && s_ResumeRuntimeStartupEvent->IsValid())
         s_ResumeRuntimeStartupEvent->Set();
 }
 

src/coreclr/src/vm/diagnosticsprotocol.h

@@ -112,7 +112,7 @@ namespace DiagnosticsIpc
      * 
      * See spec in: dotnet/diagnostics@documentation/design-docs/ipc-spec.md
      * 
-     * The flow for Advertise is a one-way burst of 24 bytes consisting of
+     * The flow for Advertise is a one-way burst of 34 bytes consisting of
      * 8 bytes  - "ADVR_V1\0" (ASCII chars + null byte)
      * 16 bytes - random 128 bit number cookie (little-endian)
      * 8 bytes  - PID (little-endian)
@@ -123,15 +123,11 @@ namespace DiagnosticsIpc
 
     const uint32_t AdvertiseSize = 34;
 
-    static GUID AdvertiseCookie_V1 = GUID_NULL;
+    // initialized in DiagnosticServer::Initialize during EEStartupHelper
+    extern GUID AdvertiseCookie_V1;
 
     inline GUID GetAdvertiseCookie_V1()
     {
-        if (AdvertiseCookie_V1 == GUID_NULL)
-        {
-            CoCreateGuid(&AdvertiseCookie_V1);
-        }
-
         return AdvertiseCookie_V1;
     }
 
@@ -143,8 +139,8 @@ namespace DiagnosticsIpc
 
         uint64_t *buffer = (uint64_t*)advertiseBuffer;
         buffer[0] = *(uint64_t*)AdvertiseMagic_V1;
-        buffer[1] = (((uint64_t)VAL32(cookie.Data1) << 32) | ((uint64_t)VAL16(cookie.Data2) << 16) | VAL16((uint64_t)cookie.Data3));
-        buffer[2] = *(uint64_t*)cookie.Data4;
+        // fills buffer[1] and buffer[2]
+        memcpy(&buffer[1], &cookie, sizeof(cookie));
         buffer[3] = VAL64(pid);
 
         // zero out unused field