From 389e7436ef1fa7d5cb2e9b18d51844d35a898cb4 Mon Sep 17 00:00:00 2001
From: reuk <reuk@users.noreply.github.com>
Date: Fri, 20 Nov 2020 12:24:13 +0000
Subject: [PATCH] IPC:  Add some missing locks in native NamedPipe
 implementations

---
 .../juce_core/native/juce_posix_NamedPipe.cpp | 16 ++++++---
 modules/juce_core/native/juce_win32_Files.cpp | 36 ++++++++++---------
 modules/juce_core/network/juce_NamedPipe.cpp  |  2 ++
 3 files changed, 33 insertions(+), 21 deletions(-)

diff --git a/modules/juce_core/native/juce_posix_NamedPipe.cpp b/modules/juce_core/native/juce_posix_NamedPipe.cpp
index 9c5a1d8fa3..e48ad25691 100644
--- a/modules/juce_core/native/juce_posix_NamedPipe.cpp
+++ b/modules/juce_core/native/juce_posix_NamedPipe.cpp
@@ -181,14 +181,20 @@ private:
 
 void NamedPipe::close()
 {
-    if (pimpl != nullptr)
     {
-        pimpl->stopReadOperation = true;
+        ScopedReadLock sl (lock);
 
-        char buffer[1] = { 0 };
-        ssize_t done = ::write (pimpl->pipeIn, buffer, 1);
-        ignoreUnused (done);
+        if (pimpl != nullptr)
+        {
+            pimpl->stopReadOperation = true;
 
+            char buffer[1] = { 0 };
+            ssize_t done = ::write (pimpl->pipeIn, buffer, 1);
+            ignoreUnused (done);
+        }
+    }
+
+    {
         ScopedWriteLock sl (lock);
         pimpl.reset();
     }
diff --git a/modules/juce_core/native/juce_win32_Files.cpp b/modules/juce_core/native/juce_win32_Files.cpp
index 04843e07ab..be764d9b8d 100644
--- a/modules/juce_core/native/juce_win32_Files.cpp
+++ b/modules/juce_core/native/juce_win32_Files.cpp
@@ -1071,14 +1071,12 @@ public:
                 return 0;
 
             OverlappedEvent over;
-            unsigned long numRead;
+            unsigned long numRead = 0;
 
             if (ReadFile (pipeH, destBuffer, (DWORD) maxBytesToRead, &numRead, &over.over))
                 return (int) numRead;
 
-            const DWORD lastError = GetLastError();
-
-            if (lastError == ERROR_IO_PENDING)
+            if (GetLastError() == ERROR_IO_PENDING)
             {
                 if (! waitForIO (over, timeOutMilliseconds))
                     return -1;
@@ -1087,7 +1085,9 @@ public:
                     return (int) numRead;
             }
 
-            if (ownsPipe && (GetLastError() == ERROR_BROKEN_PIPE || GetLastError() == ERROR_PIPE_NOT_CONNECTED))
+            const auto lastError = GetLastError();
+
+            if (ownsPipe && (lastError == ERROR_BROKEN_PIPE || lastError == ERROR_PIPE_NOT_CONNECTED))
                 disconnectPipe();
             else
                 break;
@@ -1127,7 +1127,8 @@ public:
 
     const String filename;
     HANDLE pipeH, cancelEvent;
-    bool connected, ownsPipe, shouldStop;
+    bool connected, ownsPipe;
+    std::atomic<bool> shouldStop;
     CriticalSection createFileLock;
 
 private:
@@ -1172,11 +1173,17 @@ private:
 
 void NamedPipe::close()
 {
-    if (pimpl != nullptr)
     {
-        pimpl->shouldStop = true;
-        SetEvent (pimpl->cancelEvent);
+        ScopedReadLock sl (lock);
 
+        if (pimpl != nullptr)
+        {
+            pimpl->shouldStop = true;
+            SetEvent (pimpl->cancelEvent);
+        }
+    }
+
+    {
         ScopedWriteLock sl (lock);
         pimpl.reset();
     }
@@ -1184,22 +1191,19 @@ void NamedPipe::close()
 
 bool NamedPipe::openInternal (const String& pipeName, const bool createPipe, bool mustNotExist)
 {
-    pimpl.reset (new Pimpl (pipeName, createPipe, mustNotExist));
+    auto newPimpl = std::make_unique<Pimpl> (pipeName, createPipe, mustNotExist);
 
     if (createPipe)
     {
-        if (pimpl->pipeH == INVALID_HANDLE_VALUE)
-        {
-            pimpl.reset();
+        if (newPimpl->pipeH == INVALID_HANDLE_VALUE)
             return false;
-        }
     }
-    else if (! pimpl->connect (200))
+    else if (! newPimpl->connect (200))
     {
-        pimpl.reset();
         return false;
     }
 
+    pimpl = std::move (newPimpl);
     return true;
 }
 
diff --git a/modules/juce_core/network/juce_NamedPipe.cpp b/modules/juce_core/network/juce_NamedPipe.cpp
index c1c994b292..23c84429c6 100644
--- a/modules/juce_core/network/juce_NamedPipe.cpp
+++ b/modules/juce_core/network/juce_NamedPipe.cpp
@@ -41,6 +41,7 @@ bool NamedPipe::openExisting (const String& pipeName)
 
 bool NamedPipe::isOpen() const
 {
+    ScopedReadLock sl (lock);
     return pimpl != nullptr;
 }
 
@@ -55,6 +56,7 @@ bool NamedPipe::createNewPipe (const String& pipeName, bool mustNotExist)
 
 String NamedPipe::getName() const
 {
+    ScopedReadLock sl (lock);
     return currentPipeName;
 }