Fix issue #4298 "SIGSEGV_libcoreclr.so!Debugger::GetArgCount"

The fix is to remove the call to TerminateDebugger in the EE shutdown
path. The reason was to clean up the transport pipe files but that
still happens in coreclr_uninitialize called by the host.

Also added code to clean up the transport named pipes and semaphores
on the debugger side when it detects that the target process has
terminated before it sends the ExitProcess notification.

Plumbed the cleanup call from dbi's ExitProcess code through the
native pipe line to the transport and then to pipe code.

Add PAL_CleanupTargetProcess for the "continue" named semaphore cleanup.

Found and fixed a minor race in dbgshim register runtime startup.

7 files changed, 60 insertions, 15 deletions

diff --git a/src/debug/debug-pal/unix/twowaypipe.cpp b/src/debug/debug-pal/unix/twowaypipe.cpp
index 2bf919feeb..a2304de54f 100644
--- a/src/debug/debug-pal/unix/twowaypipe.cpp
+++ b/src/debug/debug-pal/unix/twowaypipe.cpp
@@ -17,15 +17,14 @@ static const char* PipeNameFormat = "/tmp/clr-debug-pipe-%d-%llu-%s";
 
 void TwoWayPipe::GetPipeName(char *name, DWORD id, const char *suffix)
 {
-    UINT64 disambiguationKey;
-    BOOL ret = GetProcessIdDisambiguationKey(id, &disambiguationKey);
+    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 || disambiguationKey == 0);
+    _ASSERTE(ret == TRUE || m_disambiguationKey == 0);
 
-    int chars = _snprintf(name, MaxPipeNameLength, PipeNameFormat, id, disambiguationKey, suffix);
+    int chars = _snprintf(name, MaxPipeNameLength, PipeNameFormat, id, m_disambiguationKey, suffix);
     _ASSERTE(chars > 0 && chars < MaxPipeNameLength);
 }
 
@@ -201,3 +200,12 @@ bool TwoWayPipe::Disconnect()
     m_state = NotInitialized;
     return true;
 }
+
+// Used by debugger side (RS) to cleanup the target (LS) named pipes 
+// and semaphores when the debugger detects the debuggee process  exited.
+void TwoWayPipe::CleanupTargetProcess()
+{
+    unlink(m_inPipeName);
+    unlink(m_outPipeName);
+    PAL_CleanupTargetProcess(m_id, m_disambiguationKey);
+}
diff --git a/src/debug/di/dbgtransportpipeline.cpp b/src/debug/di/dbgtransportpipeline.cpp
index c117ed2c83..e3a3a8a54d 100644
--- a/src/debug/di/dbgtransportpipeline.cpp
+++ b/src/debug/di/dbgtransportpipeline.cpp
@@ -111,6 +111,13 @@ public:
     // Terminate the debuggee process.
     virtual BOOL TerminateProcess(UINT32 exitCode);
 
+#ifdef FEATURE_PAL
+    virtual void CleanupTargetProcess()
+    {
+        m_pTransport->CleanupTargetProcess();
+    }
+#endif
+
 private:
     // Return TRUE if the transport is up and runnning
     BOOL IsTransportRunning()
diff --git a/src/debug/di/nativepipeline.h b/src/debug/di/nativepipeline.h
index dd6f9ba0d0..c9560bfe65 100644
--- a/src/debug/di/nativepipeline.h
+++ b/src/debug/di/nativepipeline.h
@@ -167,6 +167,14 @@ public:
     {
         return S_FALSE;
     } 
+
+#ifdef FEATURE_PAL
+    // Used by debugger side (RS) to cleanup the target (LS) named pipes 
+    // and semaphores when the debugger detects the debuggee process  exited.
+    virtual void CleanupTargetProcess()
+    {
+    }
+#endif
 };
 
 //
diff --git a/src/debug/di/process.cpp b/src/debug/di/process.cpp
index 572eef6a75..ef7ede6450 100644
--- a/src/debug/di/process.cpp
+++ b/src/debug/di/process.cpp
@@ -14415,14 +14415,13 @@ void ExitProcessWorkItem::Do()
         PUBLIC_CALLBACK_IN_THIS_SCOPE0_NO_LOCK(GetProcess());
         pCordb->m_managedCallback->ExitProcess(GetProcess());
     }
+
     // This CordbProcess object now has no reservations against a client calling ICorDebug::Terminate.
     // That call may race against the CordbProcess::Neuter below, but since we already neutered the children,
     // that neuter call will not do anything interesting that will conflict with Terminate.
     
-    
     LOG((LF_CORDB, LL_INFO1000,"W32ET::EP: returned from ExitProcess callback\n"));
 
-
     {
         RSLockHolder ch(GetProcess()->GetStopGoLock());
 
@@ -14575,6 +14574,10 @@ void CordbWin32EventThread::ExitProcess(bool fDetach)
     // and dispatch it inband w/the other callbacks.
     if (!fDetach)
     {
+#ifdef FEATURE_PAL
+        // Cleanup the transport pipe and semaphore files that might be left by the target (LS) process.
+        m_pNativePipeline->CleanupTargetProcess();
+#endif
         ExitProcessWorkItem * pItem = new (nothrow) ExitProcessWorkItem(m_pProcess);
         if (pItem != NULL)
         {
diff --git a/src/debug/inc/dbgtransportsession.h b/src/debug/inc/dbgtransportsession.h
index 13fd2f6656..5187202753 100644
--- a/src/debug/inc/dbgtransportsession.h
+++ b/src/debug/inc/dbgtransportsession.h
@@ -319,7 +319,7 @@ public:
     // may be delivered once the session is established.
 #ifdef RIGHT_SIDE_COMPILE
     HRESULT Init(DWORD pid, HANDLE hProcessExited);
-#else // RIGHT_SIDE_COMPILE
+#else
     HRESULT Init(DebuggerIPCControlBlock * pDCB, AppDomainEnumerationIPCBlock * pADB);
 #endif // RIGHT_SIDE_COMPILE
 
@@ -331,10 +331,16 @@ public:
     // Init() may be called again to start over from the beginning).
     void Shutdown();
 
+#ifdef RIGHT_SIDE_COMPILE
+    // Used by debugger side (RS) to cleanup the target (LS) named pipes 
+    // and semaphores when the debugger detects the debuggee process  exited.
+    void CleanupTargetProcess();
+#else
     // Cleans up the named pipe connection so no tmp files are left behind. Does only
     // the minimum and must be safe to call at any time. Called during PAL ExitProcess,
     // TerminateProcess and for unhandled native exceptions and asserts.
     void AbortConnection();
+#endif // RIGHT_SIDE_COMPILE
 
     LONG AddRef()
     {
diff --git a/src/debug/inc/twowaypipe.h b/src/debug/inc/twowaypipe.h
index 6c2903440f..402ecea9b4 100644
--- a/src/debug/inc/twowaypipe.h
+++ b/src/debug/inc/twowaypipe.h
@@ -73,6 +73,10 @@ public:
         return m_state;
     }
 
+    // Used by debugger side (RS) to cleanup the target (LS) named pipes 
+    // and semaphores when the debugger detects the debuggee process  exited.
+    void CleanupTargetProcess();
+
 private:
 
     State m_state;
@@ -82,12 +86,13 @@ private:
 
     static const int MaxPipeNameLength = 64;
 
-    static void GetPipeName(char *name, DWORD id, const char *suffix);
+    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
-    char m_inPipeName[MaxPipeNameLength];  //filename of the inbound pipe
-    char m_outPipeName[MaxPipeNameLength]; //filename of the outbound pipe
+    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
 
 #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 e3827735d5..078a7ef0be 100644
--- a/src/debug/shared/dbgtransportsession.cpp
+++ b/src/debug/shared/dbgtransportsession.cpp
@@ -214,6 +214,8 @@ void DbgTransportSession::Shutdown()
     Release();
 }
 
+#ifndef RIGHT_SIDE_COMPILE
+
 // Cleans up the named pipe connection so no tmp files are left behind. Does only
 // the minimum and must be safe to call at any time. Called during PAL ExitProcess,
 // TerminateProcess and for unhandled native exceptions and asserts.
@@ -222,7 +224,6 @@ void DbgTransportSession::AbortConnection()
     m_pipe.Disconnect();
 }
 
-#ifndef RIGHT_SIDE_COMPILE
 // API used only by the LS to drive the transport into a state where it won't accept connections. This is used
 // when no proxy is detected at startup but it's too late to shutdown all of the debugging system easily. It's
 // mainly paranoia to increase the protection of your system when the proxy isn't started.
@@ -233,9 +234,16 @@ void DbgTransportSession::Neuter()
     // AV on a deallocated handle, which might happen if we simply called Shutdown()).
     m_eState = SS_Closed;
 }
-#endif // !RIGHT_SIDE_COMPILE
 
-#ifdef RIGHT_SIDE_COMPILE
+#else // RIGHT_SIDE_COMPILE
+
+// Used by debugger side (RS) to cleanup the target (LS) named pipes 
+// and semaphores when the debugger detects the debuggee process  exited.
+void DbgTransportSession::CleanupTargetProcess()
+{
+    m_pipe.CleanupTargetProcess();
+}
+
 // On the RS it may be useful to wait and see if the session can reach the SS_Open state. If the target
 // runtime has terminated for some reason then we'll never reach the open state. So the method below gives the
 // RS a way to try and establish a connection for a reasonable amount of time and to time out otherwise. They