From 5ac6af932fe2a3f4b285b6dcc79010caf807ea9d Mon Sep 17 00:00:00 2001 From: Mike McLaughlin Date: Fri, 27 May 2016 20:03:32 -0700 Subject: [PATCH] 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. --- src/debug/debug-pal/unix/twowaypipe.cpp | 26 +-- src/debug/ee/debugger.cpp | 9 - src/debug/inc/twowaypipe.h | 10 +- src/debug/shared/dbgtransportsession.cpp | 8 +- src/inc/clrconfigvalues.h | 1 - src/pal/inc/pal.h | 15 +- src/pal/src/config.h.in | 4 +- src/pal/src/configure.cmake | 76 +++++++- src/pal/src/thread/process.cpp | 320 +++++++++++++++++-------------- 9 files changed, 265 insertions(+), 204 deletions(-) diff --git a/src/debug/debug-pal/unix/twowaypipe.cpp b/src/debug/debug-pal/unix/twowaypipe.cpp index a2304de..7b2f54d 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 - #include #include #include #include #include #include - #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 eb5e65c..8634630 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 402ecea..6bc0f9f 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 078a7ef..14b509a 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; } diff --git a/src/inc/clrconfigvalues.h b/src/inc/clrconfigvalues.h index 98c5aa5..701df31 100644 --- a/src/inc/clrconfigvalues.h +++ b/src/inc/clrconfigvalues.h @@ -249,7 +249,6 @@ CONFIG_DWORD_INFO_DIRECT_ACCESS(INTERNAL_DbgTransportLog, W("DbgTransportLog"), CONFIG_DWORD_INFO_DIRECT_ACCESS(INTERNAL_DbgTransportLogClass, W("DbgTransportLogClass"), "mask to control what is logged in DbgTransportLog") RETAIL_CONFIG_STRING_INFO_EX(UNSUPPORTED_DbgTransportProxyAddress, W("DbgTransportProxyAddress"), "allows specifying the transport proxy address", CLRConfig::REGUTIL_default) CONFIG_DWORD_INFO_EX(INTERNAL_DbgTrapOnSkip, W("DbgTrapOnSkip"), 0, "allows breaking when we skip a breakpoint", CLRConfig::REGUTIL_default) -RETAIL_CONFIG_DWORD_INFO(UNSUPPORTED_DbgWaitForDebuggerAttach, W("DbgWaitForDebuggerAttach"), 0, "Makes CoreCLR wait for a managed debugger to attach on process start (1) or regular process start (0)") CONFIG_DWORD_INFO_EX(INTERNAL_DbgWaitTimeout, W("DbgWaitTimeout"), 1, "specifies the timeout value for waits", CLRConfig::REGUTIL_default) RETAIL_CONFIG_DWORD_INFO_EX(UNSUPPORTED_DbgWFDETimeout, W("DbgWFDETimeout"), 25, "specifies the timeout value for wait when waiting for a debug event", CLRConfig::REGUTIL_default) CONFIG_DWORD_INFO_EX(INTERNAL_RaiseExceptionOnAssert, W("RaiseExceptionOnAssert"), 0, "Raise a first chance (if set to 1) or second chance (if set to 2) exception on asserts.", CLRConfig::REGUTIL_default) diff --git a/src/pal/inc/pal.h b/src/pal/inc/pal.h index 08f051a..de683fd 100644 --- a/src/pal/inc/pal.h +++ b/src/pal/inc/pal.h @@ -583,12 +583,12 @@ BOOL PALAPI PAL_NotifyRuntimeStarted(); +static const int MAX_DEBUGGER_TRANSPORT_PIPE_NAME_LENGTH = 64; + PALIMPORT -VOID +void PALAPI -PAL_CleanupTargetProcess( - IN int pid, - IN UINT64 disambiguationKey); +PAL_GetTransportPipeName(char *name, DWORD id, const char *suffix); PALIMPORT void @@ -1690,13 +1690,6 @@ GetProcessTimes( OUT LPFILETIME lpKernelTime, OUT LPFILETIME lpUserTime); -PALIMPORT -BOOL -PALAPI -GetProcessIdDisambiguationKey( - IN DWORD processId, - OUT UINT64 *disambiguationKey); - #define MAXIMUM_WAIT_OBJECTS 64 #define WAIT_OBJECT_0 0 #define WAIT_ABANDONED 0x00000080 diff --git a/src/pal/src/config.h.in b/src/pal/src/config.h.in index b2c6959..75970da 100644 --- a/src/pal/src/config.h.in +++ b/src/pal/src/config.h.in @@ -101,6 +101,9 @@ #cmakedefine01 PTHREAD_CREATE_MODIFIES_ERRNO #cmakedefine01 SEM_INIT_MODIFIES_ERRNO #cmakedefine01 HAVE_PROCFS_CTL +#cmakedefine01 HAVE_PROCFS_MAPS +#cmakedefine01 HAVE_PROCFS_STAT +#cmakedefine01 HAVE_PROCFS_STATUS #cmakedefine01 HAVE_COMPATIBLE_ACOS #cmakedefine01 HAVE_COMPATIBLE_ASIN #cmakedefine01 HAVE_COMPATIBLE_POW @@ -136,7 +139,6 @@ #cmakedefine01 HAVE_SCHED_OTHER_ASSIGNABLE #define CHECK_TRACE_SPECIFIERS 0 -#define PROCFS_MEM_NAME "" #define HAVE_GETHRTIME 0 #define HAVE_LOWERCASE_ISO_NAME 0 #define HAVE_READ_REAL_TIME 0 diff --git a/src/pal/src/configure.cmake b/src/pal/src/configure.cmake index 2d630ed..4f1aabd 100644 --- a/src/pal/src/configure.cmake +++ b/src/pal/src/configure.cmake @@ -665,13 +665,85 @@ int main(void) { char path[1024]; #endif - sprintf(path, \"/proc/%u/$1\", getpid()); - fd = open(path, $2); + sprintf(path, \"/proc/%u/ctl\", getpid()); + fd = open(path, O_WRONLY); if (fd == -1) { exit(1); } exit(0); }" HAVE_PROCFS_CTL) +set(CMAKE_REQUIRED_LIBRARIES) +check_cxx_source_runs(" +#include +#include +#include +#include + +int main(void) { + int fd; +#ifdef PATH_MAX + char path[PATH_MAX]; +#elif defined(MAXPATHLEN) + char path[MAXPATHLEN]; +#else + char path[1024]; +#endif + + sprintf(path, \"/proc/%u/maps\", getpid()); + fd = open(path, O_RDONLY); + if (fd == -1) { + exit(1); + } + exit(0); +}" HAVE_PROCFS_MAPS) +set(CMAKE_REQUIRED_LIBRARIES) +check_cxx_source_runs(" +#include +#include +#include +#include + +int main(void) { + int fd; +#ifdef PATH_MAX + char path[PATH_MAX]; +#elif defined(MAXPATHLEN) + char path[MAXPATHLEN]; +#else + char path[1024]; +#endif + + sprintf(path, \"/proc/%u/stat\", getpid()); + fd = open(path, O_RDONLY); + if (fd == -1) { + exit(1); + } + exit(0); +}" HAVE_PROCFS_STAT) +set(CMAKE_REQUIRED_LIBRARIES) +check_cxx_source_runs(" +#include +#include +#include +#include + +int main(void) { + int fd; +#ifdef PATH_MAX + char path[PATH_MAX]; +#elif defined(MAXPATHLEN) + char path[MAXPATHLEN]; +#else + char path[1024]; +#endif + + sprintf(path, \"/proc/%u/status\", getpid()); + fd = open(path, O_RDONLY); + if (fd == -1) { + exit(1); + } + exit(0); +}" HAVE_PROCFS_STATUS) set(CMAKE_REQUIRED_LIBRARIES m) check_cxx_source_runs(" #include diff --git a/src/pal/src/thread/process.cpp b/src/pal/src/thread/process.cpp index 4cb5597..f0d9b2c 100644 --- a/src/pal/src/thread/process.cpp +++ b/src/pal/src/thread/process.cpp @@ -28,6 +28,7 @@ Abstract: #include "pal/process.h" #include "pal/init.h" #include "pal/critsect.h" +#include "pal/debug.h" #include "pal/dbgmsg.h" #include "pal/utils.h" #include "pal/environ.h" @@ -41,8 +42,10 @@ Abstract: #include "pal/fakepoll.h" #endif // HAVE_POLL +#include #include #include +#include #include #include #include @@ -90,6 +93,12 @@ PALAPI StartupHelperThread( LPVOID p); +static +BOOL +GetProcessIdDisambiguationKey( + IN DWORD processId, + OUT UINT64 *disambiguationKey); + // // Helper memory page used by the FlushProcessWriteBuffers // @@ -141,10 +150,6 @@ DWORD gSID = (DWORD) -1; #define CLR_SEM_MAX_NAMELEN (NAME_MAX - 4) #endif -// The runtime waits on this semaphore if the dbgshim startup semaphore exists -Volatile g_continueSem = SEM_FAILED; -char g_continueSemName[CLR_SEM_MAX_NAMELEN]; - // Function to call during PAL/process shutdown/abort Volatile g_shutdownCallback = nullptr; @@ -1433,23 +1438,23 @@ static bool IsCoreClrModule(const char* pModulePath) // Keep 31 length for Core 1.0 RC2 compatibility #if defined(__NetBSD__) static const char* RuntimeStartupSemaphoreName = "/clrst%08llx"; -static const char* RuntimeOldContinueSemaphoreName = "/clrco%08llx"; static const char* RuntimeContinueSemaphoreName = "/clrct%08llx"; #else static const char* RuntimeStartupSemaphoreName = "/clrst%08x%016llx"; -static const char* RuntimeOldContinueSemaphoreName = "/clrco%08x%016llx"; static const char* RuntimeContinueSemaphoreName = "/clrct%08x%016llx"; #endif #if defined(__NetBSD__) static uint64_t HashSemaphoreName(uint64_t a, uint64_t b) { - return (a ^ b) & 0xffffffff; + return (a ^ b) & 0xffffffff; } #else #define HashSemaphoreName(a,b) a,b #endif +static const char* PipeNameFormat = "/tmp/clr-debug-pipe-%d-%llu-%s"; + class PAL_RuntimeStartupHelper { LONG m_ref; @@ -1458,7 +1463,6 @@ class PAL_RuntimeStartupHelper PVOID m_parameter; DWORD m_threadId; HANDLE m_threadHandle; - DWORD m_processId; // A value that, used in conjunction with the process ID, uniquely identifies a process. @@ -1468,6 +1472,10 @@ class PAL_RuntimeStartupHelper // Debugger waits on this semaphore and the runtime signals it on startup. sem_t *m_startupSem; + // Debuggee waits on this semaphore and the debugger signals it after the startup callback + // registered (m_callback) returns. + sem_t *m_continueSem; + public: PAL_RuntimeStartupHelper(DWORD dwProcessId, PPAL_STARTUP_CALLBACK pfnCallback, PVOID parameter) : m_ref(1), @@ -1477,7 +1485,8 @@ public: m_threadId(0), m_threadHandle(NULL), m_processId(dwProcessId), - m_startupSem(SEM_FAILED) + m_startupSem(SEM_FAILED), + m_continueSem(SEM_FAILED) { } @@ -1496,6 +1505,19 @@ public: sem_unlink(startupSemName); } + if (m_continueSem != SEM_FAILED) + { + char continueSemName[CLR_SEM_MAX_NAMELEN]; + sprintf_s(continueSemName, + sizeof(continueSemName), + RuntimeContinueSemaphoreName, + HashSemaphoreName(m_processId, + m_processIdDisambiguationKey)); + + sem_close(m_continueSem); + sem_unlink(continueSemName); + } + if (m_threadHandle != NULL) { CloseHandle(m_threadHandle); @@ -1553,6 +1575,7 @@ public: { CPalThread *pThread = InternalGetCurrentThread(); char startupSemName[CLR_SEM_MAX_NAMELEN]; + char continueSemName[CLR_SEM_MAX_NAMELEN]; PAL_ERROR pe = NO_ERROR; // See semaphore name format for details about this value. We store it so that @@ -1567,7 +1590,23 @@ public: HashSemaphoreName(m_processId, m_processIdDisambiguationKey)); - TRACE("PAL_RuntimeStartupHelper.Register startup sem '%s'\n", startupSemName); + sprintf_s(continueSemName, + sizeof(continueSemName), + RuntimeContinueSemaphoreName, + HashSemaphoreName(m_processId, + m_processIdDisambiguationKey)); + + TRACE("PAL_RuntimeStartupHelper.Register creating startup '%s' continue '%s'\n", startupSemName, continueSemName); + + // Create the continue semaphore first so we don't race with PAL_NotifyRuntimeStarted. This open will fail if another + // debugger is trying to attach to this process because the name will already exist. + m_continueSem = sem_open(continueSemName, O_CREAT | O_EXCL, S_IRWXU, 0); + if (m_continueSem == SEM_FAILED) + { + TRACE("sem_open(continue) failed: errno is %d (%s)\n", errno, strerror(errno)); + pe = GetSemError(); + goto exit; + } // Create the debuggee startup semaphore so the runtime (debuggee) knows to wait for a debugger connection. m_startupSem = sem_open(startupSemName, O_CREAT | O_EXCL, S_IRWXU, 0); @@ -1607,6 +1646,12 @@ public: { m_canceled = true; + // Tell the runtime to continue + if (sem_post(m_continueSem) != 0) + { + ASSERT("sem_post(continueSem) failed: errno is %d (%s)\n", errno, strerror(errno)); + } + // Tell the worker thread to continue if (sem_post(m_startupSem) != 0) { @@ -1624,113 +1669,109 @@ public: } } + // + // There are a couple race conditions that need to be considered here: + // + // * On launch, between the fork and execv in the PAL's CreateProcess where the target process + // may contain a coreclr module image if the debugger process is running managed code. This + // makes just checking if the coreclr module exists not enough. + // + // * On launch (after the execv) or attach when the coreclr is loaded but before the DAC globals + // table is initialized where it is too soon to use/initialize the DAC on the debugger side. + // + // They are both fixed by check if the one of transport pipe files has been created. + // + bool IsCoreClrProcessReady() + { + char pipeName[MAX_DEBUGGER_TRANSPORT_PIPE_NAME_LENGTH]; + + PAL_GetTransportPipeName(pipeName, m_processId, "in"); + + struct stat buf; + if (stat(pipeName, &buf) == 0) + { + TRACE("IsCoreClrProcessReady: stat(%s) SUCCEEDED\n", pipeName); + return true; + } + TRACE("IsCoreClrProcessReady: stat(%s) FAILED: errno is %d (%s)\n", pipeName, errno, strerror(errno)); + return false; + } + PAL_ERROR InvokeStartupCallback() { + ProcessModules *listHead = NULL; PAL_ERROR pe = NO_ERROR; + DWORD count; - if (!m_canceled) + if (m_canceled) { - // Enumerate all the modules in the process and invoke the callback - // for the coreclr module if found. - DWORD count; - ProcessModules *listHead = CreateProcessModules(m_processId, &count); - if (listHead == NULL) - { - TRACE("CreateProcessModules failed for pid %d\n", m_processId); - pe = ERROR_INVALID_PARAMETER; - goto exit; - } + goto exit; + } - for (ProcessModules *entry = listHead; entry != NULL; entry = entry->Next) + // Enumerate all the modules in the process and invoke the callback + // for the coreclr module if found. + listHead = CreateProcessModules(m_processId, &count); + if (listHead == NULL) + { + TRACE("CreateProcessModules failed for pid %d\n", m_processId); + pe = ERROR_INVALID_PARAMETER; + goto exit; + } + + for (ProcessModules *entry = listHead; entry != NULL; entry = entry->Next) + { + if (IsCoreClrModule(entry->Name)) { - if (IsCoreClrModule(entry->Name)) + PAL_CPP_TRY { - PAL_CPP_TRY - { - TRACE("InvokeStartupCallback executing callback %p %s\n", entry->BaseAddress, entry->Name); - m_callback(entry->Name, entry->BaseAddress, m_parameter); - } - PAL_CPP_CATCH_ALL - { - } - PAL_CPP_ENDTRY - - // Currently only the first coreclr module in a process is supported - break; + TRACE("InvokeStartupCallback executing callback %p %s\n", entry->BaseAddress, entry->Name); + m_callback(entry->Name, entry->BaseAddress, m_parameter); } - } + PAL_CPP_CATCH_ALL + { + } + PAL_CPP_ENDTRY - exit: - if (listHead != NULL) - { - DestroyProcessModules(listHead); + // Currently only the first coreclr module in a process is supported + break; } } + exit: + // Wake up the runtime + if (sem_post(m_continueSem) != 0) + { + ASSERT("sem_post(continueSem) failed: errno is %d (%s)\n", errno, strerror(errno)); + } + if (listHead != NULL) + { + DestroyProcessModules(listHead); + } return pe; } void StartupHelperThread() { - char continueSemName[CLR_SEM_MAX_NAMELEN]; - sem_t *continueSem = SEM_FAILED; PAL_ERROR pe = NO_ERROR; - sprintf_s(continueSemName, - sizeof(continueSemName), - RuntimeContinueSemaphoreName, - HashSemaphoreName(m_processId, - m_processIdDisambiguationKey)); - - TRACE("StartupHelperThread continue sem '%s'\n", continueSemName); - - // Does the continue semaphore exists? If it does, the runtime is ready to be debugged. - continueSem = sem_open(continueSemName, 0); - if (continueSem != SEM_FAILED) + if (IsCoreClrProcessReady()) { - TRACE("StartupHelperThread continue sem exists - invoking callback\n"); pe = InvokeStartupCallback(); } - else if (errno == ENOENT) - { + else { + TRACE("sem_wait(startup)\n"); + // Wait until the coreclr runtime (debuggee) starts up if (sem_wait(m_startupSem) == 0) { - // The continue semaphore should exists now and is needed to wake up the runtimes below - continueSem = sem_open(continueSemName, 0); - if (continueSem != SEM_FAILED) - { - TRACE("StartupHelperThread continue sem exists after wait - invoking callback\n"); - pe = InvokeStartupCallback(); - } - else - { - TRACE("sem_open(continue) failed: errno is %d (%s)\n", errno, strerror(errno)); - pe = GetSemError(); - } + pe = InvokeStartupCallback(); } - else + else { TRACE("sem_wait(startup) failed: errno is %d (%s)\n", errno, strerror(errno)); pe = GetSemError(); } } - else - { - pe = GetSemError(); - } - - // Wake up the runtime even on error and cancelation - if (continueSem != SEM_FAILED) - { - if (sem_post(continueSem) != 0) - { - TRACE("sem_post(continueSem) failed: errno is %d (%s)\n", errno, strerror(errno)); - pe = GetSemError(); - } - - sem_close(continueSem); - } // Invoke the callback on errors if (pe != NO_ERROR && !m_canceled) @@ -1845,35 +1886,19 @@ PALAPI PAL_NotifyRuntimeStarted() { char startupSemName[CLR_SEM_MAX_NAMELEN]; + char continueSemName[CLR_SEM_MAX_NAMELEN]; sem_t *startupSem = SEM_FAILED; + sem_t *continueSem = SEM_FAILED; BOOL result = TRUE; UINT64 processIdDisambiguationKey = 0; GetProcessIdDisambiguationKey(gPID, &processIdDisambiguationKey); sprintf_s(startupSemName, sizeof(startupSemName), RuntimeStartupSemaphoreName, HashSemaphoreName(gPID, processIdDisambiguationKey)); - sprintf_s(g_continueSemName, sizeof(g_continueSemName), RuntimeOldContinueSemaphoreName, HashSemaphoreName(gPID, processIdDisambiguationKey)); + sprintf_s(continueSemName, sizeof(continueSemName), RuntimeContinueSemaphoreName, HashSemaphoreName(gPID, processIdDisambiguationKey)); - TRACE("PAL_NotifyRuntimeStarted opening continue (old) '%s' startup '%s'\n", g_continueSemName, startupSemName); + TRACE("PAL_NotifyRuntimeStarted opening continue '%s' startup '%s'\n", continueSemName, startupSemName); - // For backwards compatibility with RC2 (see issue #4410) first OPEN the continue semaphore with the old name "clrcoXXXX". - g_continueSem = sem_open(g_continueSemName, 0); - if (g_continueSem == SEM_FAILED) - { - // Create the new continue semaphore name "clrctXXXX" - sprintf_s(g_continueSemName, sizeof(g_continueSemName), RuntimeContinueSemaphoreName, HashSemaphoreName(gPID, processIdDisambiguationKey)); - - TRACE("PAL_NotifyRuntimeStarted creating continue '%s'\n", g_continueSemName); - - // Create the continue semaphore. This tells dbgshim that coreclr is initialized and ready. - g_continueSem = sem_open(g_continueSemName, O_CREAT | O_EXCL, S_IRWXU, 0); - if (g_continueSem == SEM_FAILED) - { - ASSERT("sem_open(%s) failed: %d (%s)\n", g_continueSemName, errno, strerror(errno)); - result = FALSE; - goto exit; - } - } // Open the debugger startup semaphore. If it doesn't exists, then we do nothing and // the function is successful. @@ -1884,6 +1909,14 @@ PAL_NotifyRuntimeStarted() goto exit; } + continueSem = sem_open(continueSemName, 0); + if (continueSem == SEM_FAILED) + { + ASSERT("sem_open(%s) failed: %d (%s)\n", continueSemName, errno, strerror(errno)); + result = FALSE; + goto exit; + } + // Wake up the debugger waiting for startup if (sem_post(startupSem) != 0) { @@ -1893,7 +1926,7 @@ PAL_NotifyRuntimeStarted() } // Now wait until the debugger's runtime startup notification is finished - if (sem_wait(g_continueSem) != 0) + if (sem_wait(continueSem) != 0) { ASSERT("sem_wait(continueSem) failed: errno is %d (%s)\n", errno, strerror(errno)); result = FALSE; @@ -1905,41 +1938,14 @@ exit: { sem_close(startupSem); } + if (continueSem != SEM_FAILED) + { + sem_close(continueSem); + } return result; } /*++ - PAL_CleanupTargetProcess - - Cleanup the target process's name continue semaphore - on the debugger side when the debugger detects the - process termination. - -Parameters: - pid - process id - disambiguationKey - key to make process id unique - -Return value: - None ---*/ -VOID -PALAPI -PAL_CleanupTargetProcess( - IN int pid, - IN UINT64 disambiguationKey) -{ - char continueSemName[NAME_MAX - 4]; - - sprintf_s(continueSemName, - sizeof(continueSemName), - RuntimeContinueSemaphoreName, - pid, - disambiguationKey); - - sem_unlink(continueSemName); -} - -/*++ Function: GetProcessIdDisambiguationKey @@ -2012,7 +2018,7 @@ GetProcessIdDisambiguationKey(DWORD processId, UINT64 *disambiguationKey) return TRUE; -#elif defined(HAVE_PROCFS_CTL) +#elif HAVE_PROCFS_STAT // Here we read /proc//stat file to get the start time for the process. // We return this value (which is expressed in jiffies since boot time). @@ -2066,12 +2072,34 @@ GetProcessIdDisambiguationKey(DWORD processId, UINT64 *disambiguationKey) #else // If this is not OS X and we don't have /proc, we just return FALSE. - WARN(!"GetProcessIdDisambiguationKey was called but is not implemented on this platform!"); + WARN("GetProcessIdDisambiguationKey was called but is not implemented on this platform!"); return FALSE; #endif } /*++ + Function: + PAL_GetTransportPipeName + + Builds the transport pipe names from the process id. +--*/ +void +PALAPI +PAL_GetTransportPipeName(char *name, DWORD id, const char *suffix) +{ + UINT64 disambiguationKey = 0; + BOOL ret = GetProcessIdDisambiguationKey(id, &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); + + int chars = _snprintf(name, MAX_DEBUGGER_TRANSPORT_PIPE_NAME_LENGTH, PipeNameFormat, id, disambiguationKey, suffix); + _ASSERTE(chars > 0 && chars < MAX_DEBUGGER_TRANSPORT_PIPE_NAME_LENGTH); +} + +/*++ Function: GetProcessTimes @@ -2698,8 +2726,9 @@ CreateProcessModules( free(line); // We didn't allocate line, but as per contract of getline we should free it pclose(vmmapFile); +exit: -#elif defined(HAVE_PROCFS_CTL) +#elif HAVE_PROCFS_MAPS // Here we read /proc//maps file in order to parse it and figure out what it says // about a library we are looking for. This file looks something like this: @@ -2778,10 +2807,11 @@ CreateProcessModules( free(line); // We didn't allocate line, but as per contract of getline we should free it fclose(mapsFile); +exit: + #else _ASSERTE(!"Not implemented on this platform"); #endif -exit: return listHead; } @@ -2825,14 +2855,6 @@ void PROCNotifyProcessShutdown() { callback(); } - - // Cleanup the name continue semaphore on exit and abormal terminatation - sem_t *continueSem = InterlockedExchangePointer(&g_continueSem, SEM_FAILED); - if (continueSem != SEM_FAILED) - { - sem_close(continueSem); - sem_unlink(g_continueSemName); - } } /*++ -- 2.7.4