From a8624052c9e7b759e7f943cb2a84a2520bef1b79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Rivero?= Date: Thu, 21 Mar 2019 09:08:46 -0700 Subject: [EventPipe] Fix linker warning on Debug/Checked build and unlink Unix Domain Socket #23334 (#23357) - Attempt to close-behind unix domain socket and repurpose LF_REMOTING facility. - On shutdown, attempt to unlink the bound socket so it can be removed from the file system when the last reference to it it closed. - Rename unused LF_REMOTING to LF_DIAGNOSTICS_PORT. This new flag will be used by the diagnostic server stress log calls. - libcmtd.lib(initializers.obj) : warning LNK4098: defaultlib 'libcmt.lib' conflicts with use of other libs; use /NODEFAULTLIB:library [S:\github.com\jorive\coreclr\bin\obj\Windows_NT.x64.Checked\src\dlls\mscoree\coreclr\coreclr.vcxproj] - Move some preprocessors and includes around. --- src/debug/debug-pal/CMakeLists.txt | 4 +-- src/debug/debug-pal/unix/diagnosticsipc.cpp | 51 +++++++++++++++++++++++++---- src/debug/debug-pal/win/diagnosticsipc.cpp | 4 +++ src/debug/inc/diagnosticsipc.h | 19 ++++++++--- 4 files changed, 65 insertions(+), 13 deletions(-) (limited to 'src/debug') diff --git a/src/debug/debug-pal/CMakeLists.txt b/src/debug/debug-pal/CMakeLists.txt index c1b5b443a9..ac0f97e468 100644 --- a/src/debug/debug-pal/CMakeLists.txt +++ b/src/debug/debug-pal/CMakeLists.txt @@ -5,12 +5,10 @@ include_directories(../../pal/inc) add_definitions(-DPAL_STDCPP_COMPAT=1) if(WIN32) - #use static crt - add_definitions(-MT) add_definitions(-DWIN32_LEAN_AND_MEAN) include_directories(../../inc) #needed for warning control - set(TWO_WAY_PIPE_SOURCES + set(TWO_WAY_PIPE_SOURCES win/diagnosticsipc.cpp win/twowaypipe.cpp win/processdescriptor.cpp diff --git a/src/debug/debug-pal/unix/diagnosticsipc.cpp b/src/debug/debug-pal/unix/diagnosticsipc.cpp index 182dff01be..d9e0e94c9f 100644 --- a/src/debug/debug-pal/unix/diagnosticsipc.cpp +++ b/src/debug/debug-pal/unix/diagnosticsipc.cpp @@ -7,7 +7,6 @@ #include #include #include -#include #include #include #include "diagnosticsipc.h" @@ -15,7 +14,8 @@ IpcStream::DiagnosticsIpc::DiagnosticsIpc(const int serverSocket, sockaddr_un *const pServerAddress) : _serverSocket(serverSocket), - _pServerAddress(new (std::nothrow) sockaddr_un) + _pServerAddress(new (std::nothrow) sockaddr_un), + _isUnlinked(false) { _ASSERTE(_pServerAddress != nullptr); _ASSERTE(_serverSocket != -1); @@ -33,8 +33,7 @@ IpcStream::DiagnosticsIpc::~DiagnosticsIpc() const int fSuccessClose = ::close(_serverSocket); _ASSERTE(fSuccessClose != -1); // TODO: Add error handling? - const int fSuccessUnlink = ::unlink(_pServerAddress->sun_path); - _ASSERTE(fSuccessUnlink != -1); // TODO: Add error handling? + Unlink(); delete _pServerAddress; } @@ -68,6 +67,26 @@ IpcStream::DiagnosticsIpc *IpcStream::DiagnosticsIpc::Create(const char *const p if (callback != nullptr) callback(strerror(errno), errno); _ASSERTE(fSuccessBind != -1); + + const int fSuccessClose = ::close(serverSocket); + _ASSERTE(fSuccessClose != -1); + + return nullptr; + } + + const int fSuccessfulListen = ::listen(serverSocket, /* backlog */ 255); + if (fSuccessfulListen == -1) + { + if (callback != nullptr) + callback(strerror(errno), errno); + _ASSERTE(fSuccessfulListen != -1); + + const int fSuccessUnlink = ::unlink(serverAddress.sun_path); + _ASSERTE(fSuccessUnlink != -1); + + const int fSuccessClose = ::close(serverSocket); + _ASSERTE(fSuccessClose != -1); + return nullptr; } @@ -76,8 +95,6 @@ IpcStream::DiagnosticsIpc *IpcStream::DiagnosticsIpc::Create(const char *const p IpcStream *IpcStream::DiagnosticsIpc::Accept(ErrorCallback callback) const { - if (::listen(_serverSocket, /* backlog */ 255) == -1) - return nullptr; sockaddr_un from; socklen_t fromlen = sizeof(from); const int clientSocket = ::accept(_serverSocket, (sockaddr *)&from, &fromlen); @@ -94,6 +111,28 @@ IpcStream *IpcStream::DiagnosticsIpc::Accept(ErrorCallback callback) const return pIpcStream; } +//! This helps remove the socket from the filesystem when the runtime exits. +//! From: http://man7.org/linux/man-pages/man7/unix.7.html#NOTES +//! Binding to a socket with a filename creates a socket in the +//! filesystem that must be deleted by the caller when it is no longer +//! needed (using unlink(2)). The usual UNIX close-behind semantics +//! apply; the socket can be unlinked at any time and will be finally +//! removed from the filesystem when the last reference to it is closed. +void IpcStream::DiagnosticsIpc::Unlink(ErrorCallback callback) +{ + if (_isUnlinked) + return; + _isUnlinked = true; + + const int fSuccessUnlink = ::unlink(_pServerAddress->sun_path); + if (fSuccessUnlink == -1) + { + if (callback != nullptr) + callback(strerror(errno), errno); + _ASSERTE(fSuccessUnlink == 0); + } +} + IpcStream::~IpcStream() { if (_clientSocket != -1) diff --git a/src/debug/debug-pal/win/diagnosticsipc.cpp b/src/debug/debug-pal/win/diagnosticsipc.cpp index 7d9f84e7e5..46f212836b 100644 --- a/src/debug/debug-pal/win/diagnosticsipc.cpp +++ b/src/debug/debug-pal/win/diagnosticsipc.cpp @@ -77,6 +77,10 @@ IpcStream *IpcStream::DiagnosticsIpc::Accept(ErrorCallback callback) const return pIpcStream; } +void IpcStream::DiagnosticsIpc::Unlink(ErrorCallback callback) +{ +} + IpcStream::~IpcStream() { if (_hPipe != INVALID_HANDLE_VALUE) diff --git a/src/debug/inc/diagnosticsipc.h b/src/debug/inc/diagnosticsipc.h index 81789f0803..54277e1af9 100644 --- a/src/debug/inc/diagnosticsipc.h +++ b/src/debug/inc/diagnosticsipc.h @@ -26,20 +26,31 @@ public: class DiagnosticsIpc final { public: - static DiagnosticsIpc *Create(const char *const pIpcName, ErrorCallback callback = nullptr); ~DiagnosticsIpc(); + + //! Creates an IPC object + static DiagnosticsIpc *Create(const char *const pIpcName, ErrorCallback callback = nullptr); + + //! Enables the underlaying IPC implementation to accept connection. IpcStream *Accept(ErrorCallback callback = nullptr) const; + //! Used to unlink the socket so it can be removed from the filesystem + //! when the last reference to it is closed. + void Unlink(ErrorCallback callback = nullptr); + private: #ifdef FEATURE_PAL - DiagnosticsIpc(const int serverSocket, sockaddr_un *const pServerAddress); - const int _serverSocket = -1; + const int _serverSocket; sockaddr_un *const _pServerAddress; + bool _isUnlinked = false; + + DiagnosticsIpc(const int serverSocket, sockaddr_un *const pServerAddress); #else static const uint32_t MaxNamedPipeNameLength = 256; - DiagnosticsIpc(const char(&namedPipeName)[MaxNamedPipeNameLength]); char _pNamedPipeName[MaxNamedPipeNameLength]; // https://docs.microsoft.com/en-us/windows/desktop/api/winbase/nf-winbase-createnamedpipea + + DiagnosticsIpc(const char(&namedPipeName)[MaxNamedPipeNameLength]); #endif /* FEATURE_PAL */ DiagnosticsIpc() = delete; -- cgit v1.2.3