Fix the named semaphore leak on OSX (and Linux) (#5269)

* Change the dbgshim launch handshake back.

The debugger side now creates the name semaphores like before and the transport pipe
existence determines that coreclr is ready.

Changed when the transport pipes are created: synchronously on the main thread.

Correctly set and check the HAVE_PROCFS_* defines.

* Code review feedback.

4 files changed, 13 insertions, 40 deletions

diff --git a/src/debug/debug-pal/unix/twowaypipe.cpp b/src/debug/debug-pal/unix/twowaypipe.cpp
index a2304de54f..7b2f54d8a1 100644
--- a/src/debug/debug-pal/unix/twowaypipe.cpp
+++ b/src/debug/debug-pal/unix/twowaypipe.cpp
@@ -3,31 +3,14 @@
 // See the LICENSE file in the project root for more information.
 
 #include <pal.h>
-
 #include <unistd.h>
 #include <fcntl.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <limits.h>
 #include <pal_assert.h>
-
 #include "twowaypipe.h"
 
-static const char* PipeNameFormat = "/tmp/clr-debug-pipe-%d-%llu-%s";
-
-void TwoWayPipe::GetPipeName(char *name, DWORD id, const char *suffix)
-{
-    BOOL ret = GetProcessIdDisambiguationKey(id, &m_disambiguationKey);
-
-    // If GetProcessIdDisambiguationKey failed for some reason, it should set the value 
-    // to 0. We expect that anyone else making the pipe name will also fail and thus will
-    // also try to use 0 as the value.
-    _ASSERTE(ret == TRUE || m_disambiguationKey == 0);
-
-    int chars = _snprintf(name, MaxPipeNameLength, PipeNameFormat, id, m_disambiguationKey, suffix);
-    _ASSERTE(chars > 0 && chars < MaxPipeNameLength);
-}
-
 // Creates a server side of the pipe. 
 // Id is used to create pipes names and uniquely identify the pipe on the machine. 
 // true - success, false - failure (use GetLastError() for more details)
@@ -38,8 +21,8 @@ bool TwoWayPipe::CreateServer(DWORD id)
         return false;
 
     m_id = id;
-    GetPipeName(m_inPipeName, id, "in");
-    GetPipeName(m_outPipeName, id, "out");
+    PAL_GetTransportPipeName(m_inPipeName, id, "in");
+    PAL_GetTransportPipeName(m_outPipeName, id, "out");
 
     if (mkfifo(m_inPipeName, S_IRWXU) == -1)
     {
@@ -67,8 +50,8 @@ bool TwoWayPipe::Connect(DWORD id)
 
     m_id = id;
     //"in" and "out" are switched deliberately, because we're on the client
-    GetPipeName(m_inPipeName, id, "out");
-    GetPipeName(m_outPipeName, id, "in");
+    PAL_GetTransportPipeName(m_inPipeName, id, "out");
+    PAL_GetTransportPipeName(m_outPipeName, id, "in");
 
     // Pipe opening order is reversed compared to WaitForConnection()
     // in order to avaid deadlock.
@@ -207,5 +190,4 @@ void TwoWayPipe::CleanupTargetProcess()
 {
     unlink(m_inPipeName);
     unlink(m_outPipeName);
-    PAL_CleanupTargetProcess(m_id, m_disambiguationKey);
 }
diff --git a/src/debug/ee/debugger.cpp b/src/debug/ee/debugger.cpp
index eb5e65c415..8634630ebe 100644
--- a/src/debug/ee/debugger.cpp
+++ b/src/debug/ee/debugger.cpp
@@ -2113,18 +2113,9 @@ HRESULT Debugger::Startup(void)
             ShutdownTransport();
             ThrowHR(hr);
         }
-
     #ifdef FEATURE_PAL
         PAL_SetShutdownCallback(AbortTransport);
     #endif // FEATURE_PAL
-
-         bool waitForAttach = CLRConfig::GetConfigValue(CLRConfig::UNSUPPORTED_DbgWaitForDebuggerAttach) != 0;
-         if (waitForAttach)
-         {
-             // Mark this process as launched by the debugger and the debugger as attached.
-             g_CORDebuggerControlFlags |= DBCF_GENERATE_DEBUG_CODE;
-             MarkDebuggerAttachedInternal();
-         }
     #endif // FEATURE_DBGIPC_TRANSPORT_VM
 
         RaiseStartupNotification();
diff --git a/src/debug/inc/twowaypipe.h b/src/debug/inc/twowaypipe.h
index 402ecea9b4..6bc0f9f39e 100644
--- a/src/debug/inc/twowaypipe.h
+++ b/src/debug/inc/twowaypipe.h
@@ -81,18 +81,12 @@ private:
 
     State m_state;
 
-
 #ifdef FEATURE_PAL
 
-    static const int MaxPipeNameLength = 64;
-
-    void GetPipeName(char *name, DWORD id, const char *suffix);
-
     int m_id;                               // id that was passed to CreateServer() or Connect()
     int m_inboundPipe, m_outboundPipe;      // two one sided pipes used for communication
-    UINT64 m_disambiguationKey;             // key to make the names more unique
-    char m_inPipeName[MaxPipeNameLength];   // filename of the inbound pipe
-    char m_outPipeName[MaxPipeNameLength];  // filename of the outbound pipe
+    char m_inPipeName[MAX_DEBUGGER_TRANSPORT_PIPE_NAME_LENGTH];   // filename of the inbound pipe
+    char m_outPipeName[MAX_DEBUGGER_TRANSPORT_PIPE_NAME_LENGTH];  // filename of the outbound pipe
 
 #else
     // Connects to a one sided pipe previously created by CreateOneWayPipe.
diff --git a/src/debug/shared/dbgtransportsession.cpp b/src/debug/shared/dbgtransportsession.cpp
index 078a7ef0be..14b509aa9c 100644
--- a/src/debug/shared/dbgtransportsession.cpp
+++ b/src/debug/shared/dbgtransportsession.cpp
@@ -130,6 +130,11 @@ HRESULT DbgTransportSession::Init(DebuggerIPCControlBlock *pDCB, AppDomainEnumer
     m_hSessionOpenEvent = WszCreateEvent(NULL, TRUE, FALSE, NULL); // Manual reset, not signalled
     if (m_hSessionOpenEvent == NULL)
         return E_OUTOFMEMORY;
+#else // RIGHT_SIDE_COMPILE
+    DWORD pid = GetCurrentProcessId(); 
+    if (!m_pipe.CreateServer(pid)) {
+        return E_OUTOFMEMORY;
+    }
 #endif // RIGHT_SIDE_COMPILE
 
     // Allocate some buffers to receive incoming events. The initial number is chosen arbitrarily, tune as
@@ -1345,7 +1350,8 @@ void DbgTransportSession::TransportWorker()
         else
         {
             DWORD pid = GetCurrentProcessId(); 
-            if (m_pipe.CreateServer(pid) && m_pipe.WaitForConnection())
+            if ((m_pipe.GetState() == TwoWayPipe::Created || m_pipe.CreateServer(pid)) && 
+                 m_pipe.WaitForConnection())
             {
                 eStatus = SCS_Success;
             }