core.git: sal/osl

[Noel Grandin (via logerrit)]

 sal/osl/unx/pipe.cxx     |   50 +++++++++++++++++++++++++++++++++--------------
 sal/osl/unx/sockimpl.hxx |    2 +
 2 files changed, 38 insertions(+), 14 deletions(-)

New commits:
commit c06de26c4b83edad29a70a454dea20dcd0261a33
Author:     Noel Grandin <noel.gran...@collabora.co.uk>
AuthorDate: Thu Nov 21 16:17:16 2024 +0200
Commit:     Noel Grandin <noelgran...@gmail.com>
CommitDate: Fri Nov 22 08:53:40 2024 +0100

    tsan: make oslPipe thread-safe
    
    add a mutex around accesses of the internal data structures of
    oslPipeImpl, to prevent data-races
    
    Change-Id: Ia39961c0a3df0a5a5db31cc515d248d013998255
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/176931
    Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>
    Tested-by: Jenkins
    Reviewed-by: Stephan Bergmann <stephan.bergm...@allotropia.de>

diff --git a/sal/osl/unx/pipe.cxx b/sal/osl/unx/pipe.cxx
index 436f3086ded5..c4420731795d 100644
--- a/sal/osl/unx/pipe.cxx
+++ b/sal/osl/unx/pipe.cxx
@@ -82,12 +82,10 @@ static oslPipeError osl_PipeErrorFromNative(int nativeType)
 
 static oslPipe createPipeImpl()
 {
-    oslPipe pPipeImpl;
-
-    pPipeImpl = static_cast< oslPipe >(calloc(1, sizeof(struct oslPipeImpl)));
-    if (!pPipeImpl)
-        return nullptr;
+    oslPipe pPipeImpl = new oslPipeImpl;
 
+    pPipeImpl->m_Socket = 0;
+    pPipeImpl->m_Name[0] = 0;
     pPipeImpl->m_nRefCount = 1;
     pPipeImpl->m_bClosed = false;
 #if defined(CLOSESOCKET_DOESNT_WAKE_UP_ACCEPT)
@@ -100,8 +98,7 @@ static oslPipe createPipeImpl()
 
 static void destroyPipeImpl(oslPipe pImpl)
 {
-    if (pImpl)
-        free(pImpl);
+    delete pImpl;
 }
 
 oslPipe SAL_CALL osl_createPipe(rtl_uString *ustrPipeName, oslPipeOptions 
Options, oslSecurity Security)
@@ -296,8 +293,7 @@ void SAL_CALL osl_releasePipe(oslPipe pPipe)
 
     if (osl_atomic_decrement(&(pPipe->m_nRefCount)) == 0)
     {
-        if (!pPipe->m_bClosed)
-            osl_closePipe(pPipe);
+        osl_closePipe(pPipe);
 
         destroyPipeImpl(pPipe);
     }
@@ -311,6 +307,8 @@ void SAL_CALL osl_closePipe(oslPipe pPipe)
     if (!pPipe)
         return;
 
+    std::unique_lock aGuard(pPipe->m_Mutex);
+
     if (pPipe->m_bClosed)
         return;
 
@@ -373,13 +371,23 @@ oslPipe SAL_CALL osl_acceptPipe(oslPipe pPipe)
     if (!pPipe)
         return nullptr;
 
-    assert(pPipe->m_Name[0] != '
+    int socket;
+    {
+        // dont hold lock while accepting, so it is possible to close a socket 
blocked in accept
+        std::unique_lock aGuard(pPipe->m_Mutex);
 
+        assert(pPipe->m_Name[0] != '
 #if defined(CLOSESOCKET_DOESNT_WAKE_UP_ACCEPT)
-    pPipe->m_bIsAccepting = true;
+        pPipe->m_bIsAccepting = true;
 #endif
 
-    s = accept(pPipe->m_Socket, nullptr, nullptr);
+        socket = pPipe->m_Socket;
+    }
+
+
+    s = accept(socket, nullptr, nullptr);
+
+    std::unique_lock aGuard(pPipe->m_Mutex);
 
 #if defined(CLOSESOCKET_DOESNT_WAKE_UP_ACCEPT)
     pPipe->m_bIsAccepting = false;
@@ -435,7 +443,14 @@ sal_Int32 SAL_CALL osl_receivePipe(oslPipe pPipe,
         return -1;
     }
 
-    sal_Int32 nRet = recv(pPipe->m_Socket, pBuffer, BytesToRead, 0);
+    int socket;
+    {
+        // dont hold lock while receiving, so it is possible to close a socket 
blocked in recv
+        std::unique_lock aGuard(pPipe->m_Mutex);
+        socket = pPipe->m_Socket;
+    }
+
+    sal_Int32 nRet = recv(socket, pBuffer, BytesToRead, 0);
 
     SAL_WARN_IF(nRet < 0, "sal.osl.pipe", "recv() failed: " << 
UnixErrnoString(errno));
 
@@ -456,7 +471,14 @@ sal_Int32 SAL_CALL osl_sendPipe(oslPipe pPipe,
         return -1;
     }
 
-    nRet = send(pPipe->m_Socket, pBuffer, BytesToSend, 0);
+    int socket;
+    {
+        // dont hold lock while sending, so it is possible to close a socket 
blocked in send
+        std::unique_lock aGuard(pPipe->m_Mutex);
+        socket = pPipe->m_Socket;
+    }
+
+    nRet = send(socket, pBuffer, BytesToSend, 0);
 
     if (nRet <= 0)
         SAL_WARN("sal.osl.pipe", "send() failed: " << UnixErrnoString(errno));
diff --git a/sal/osl/unx/sockimpl.hxx b/sal/osl/unx/sockimpl.hxx
index dc354db94a42..529aec7ff1eb 100644
--- a/sal/osl/unx/sockimpl.hxx
+++ b/sal/osl/unx/sockimpl.hxx
@@ -24,6 +24,7 @@
 
 #include <sys/socket.h>
 #include <sys/un.h>
+#include <mutex>
 
 #if defined(LINUX) || defined(FREEBSD) || defined(NETBSD)
 #define CLOSESOCKET_DOESNT_WAKE_UP_ACCEPT 1
@@ -45,6 +46,7 @@ struct oslSocketAddrImpl
 };
 
 struct oslPipeImpl {
+    std::mutex m_Mutex;
     int  m_Socket;
     char m_Name[sizeof sockaddr_un::sun_path];
     oslInterlockedCount m_nRefCount;