Fixed an issue when connecting to a non-existent NamedPipe

juce-framework · Sep 19, 2018 · 05f49ba · 05f49ba
1 parent c9d05ca
commit 05f49ba
Show file tree

Hide file tree

Showing 3 changed files with 235 additions and 17 deletions.
diff --git a/modules/juce_core/native/juce_posix_NamedPipe.cpp b/modules/juce_core/native/juce_posix_NamedPipe.cpp
@@ -47,18 +47,14 @@ class NamedPipe::Pimpl
         }
     }
 
+    bool connect (int timeOutMilliseconds)
+    {
+        return openPipe (true, getTimeoutEnd (timeOutMilliseconds));
+    }
+
     int read (char* destBuffer, int maxBytesToRead, int timeOutMilliseconds)
     {
         auto timeoutEnd = getTimeoutEnd (timeOutMilliseconds);
-
-        if (pipeIn == -1)
-        {
-            pipeIn = openPipe (createdPipe ? pipeInName : pipeOutName, O_RDWR | O_NONBLOCK, timeoutEnd);
-
-            if (pipeIn == -1)
-                return -1;
-        }
-
         int bytesRead = 0;
 
         while (bytesRead < maxBytesToRead)
@@ -89,13 +85,8 @@ class NamedPipe::Pimpl
     {
         auto timeoutEnd = getTimeoutEnd (timeOutMilliseconds);
 
-        if (pipeOut == -1)
-        {
-            pipeOut = openPipe (createdPipe ? pipeOutName : pipeInName, O_WRONLY, timeoutEnd);
-
-            if (pipeOut == -1)
-                return -1;
-        }
+        if (! openPipe (false, timeoutEnd))
+            return -1;
 
         int bytesWritten = 0;
 
@@ -160,6 +151,25 @@ class NamedPipe::Pimpl
         }
     }
 
+    bool openPipe (bool isInput, uint32 timeoutEnd)
+    {
+        auto& pipe = isInput ? pipeIn : pipeOut;
+        int flags = isInput ? O_RDWR | O_NONBLOCK : O_WRONLY;
+
+        const String& pipeName = isInput ? (createdPipe ? pipeInName : pipeOutName)
+                                         : (createdPipe ? pipeOutName : pipeInName);
+
+        if (pipe == -1)
+        {
+            pipe = openPipe (pipeName, flags, timeoutEnd);
+
+            if (pipe == -1)
+                return false;
+        }
+
+        return true;
+    }
+
     static void waitForInput (int handle, int timeoutMsecs) noexcept
     {
         struct timeval timeout;
@@ -211,6 +221,12 @@ bool NamedPipe::openInternal (const String& pipeName, bool createPipe, bool must
         return false;
     }
 
+    if (! pimpl->connect (200))
+    {
+        pimpl.reset();
+        return false;
+    }
+
     return true;
 }
 

diff --git a/modules/juce_core/native/juce_win32_Files.cpp b/modules/juce_core/native/juce_win32_Files.cpp
@@ -1180,7 +1180,15 @@ bool NamedPipe::openInternal (const String& pipeName, const bool createPipe, boo
 {
     pimpl.reset (new Pimpl (pipeName, createPipe, mustNotExist));
 
-    if (createPipe && pimpl->pipeH == INVALID_HANDLE_VALUE)
+    if (createPipe)
+    {
+        if (pimpl->pipeH == INVALID_HANDLE_VALUE)
+        {
+            pimpl.reset();
+            return false;
+        }
+    }
+    else if (! pimpl->connect (200))
     {
         pimpl.reset();
         return false;

diff --git a/modules/juce_core/network/juce_NamedPipe.cpp b/modules/juce_core/network/juce_NamedPipe.cpp
@@ -60,4 +60,198 @@ String NamedPipe::getName() const
 
 // other methods for this class are implemented in the platform-specific files
 
+//==============================================================================
+
+#if JUCE_UNIT_TESTS
+
+class NamedPipeTests  : public UnitTest
+{
+public:
+    //==============================================================================
+    NamedPipeTests()
+        : UnitTest ("NamedPipe", "Networking")
+    {}
+
+    void runTest() override
+    {
+        const String pipeName ("TestPipe");
+
+        beginTest ("Pre test cleanup");
+        {
+            NamedPipe pipe;
+            expect (pipe.createNewPipe (pipeName, false));
+        }
+
+        beginTest ("Create pipe");
+        {
+            NamedPipe pipe;
+            expect (! pipe.isOpen());
+
+            expect (pipe.createNewPipe (pipeName, true));
+            expect (pipe.isOpen());
+
+            expect (pipe.createNewPipe (pipeName, false));
+            expect (pipe.isOpen());
+
+            NamedPipe otherPipe;
+            expect (! otherPipe.createNewPipe (pipeName, true));
+            expect (! otherPipe.isOpen());
+        }
+
+        beginTest ("Existing pipe");
+        {
+            NamedPipe pipe;
+
+            expect (! pipe.openExisting (pipeName));
+            expect (! pipe.isOpen());
+
+            expect (pipe.createNewPipe (pipeName, true));
+
+            NamedPipe otherPipe;
+            expect (otherPipe.openExisting (pipeName));
+            expect (otherPipe.isOpen());
+        }
+
+        int sendData = 4684682;
+
+        beginTest ("Receive message created pipe");
+        {
+            NamedPipe pipe;
+            expect (pipe.createNewPipe (pipeName, true));
+
+            WaitableEvent senderFinished;
+            SenderThread sender (pipeName, false, senderFinished, sendData);
+
+            sender.startThread();
+
+            int recvData = -1;
+            auto bytesRead = pipe.read (&recvData, sizeof (recvData), 2000);
+
+            expect (senderFinished.wait (4000));
+
+            expectEquals (bytesRead, (int) sizeof (recvData));
+            expectEquals (sender.result, (int) sizeof (sendData));
+            expectEquals (recvData, sendData);
+        }
+
+        beginTest ("Receive message existing pipe");
+        {
+            WaitableEvent senderFinished;
+            SenderThread sender (pipeName, true, senderFinished, sendData);
+
+            NamedPipe pipe;
+            expect (pipe.openExisting (pipeName));
+
+            sender.startThread();
+
+            int recvData = -1;
+            auto bytesRead = pipe.read (&recvData, sizeof (recvData), 2000);
+
+            expect (senderFinished.wait (4000));
+
+            expectEquals (bytesRead, (int) sizeof (recvData));
+            expectEquals (sender.result, (int) sizeof (sendData));
+            expectEquals (recvData, sendData);
+        }
+
+        beginTest ("Send message created pipe");
+        {
+            NamedPipe pipe;
+            expect (pipe.createNewPipe (pipeName, true));
+
+            WaitableEvent receiverFinished;
+            ReceiverThread receiver (pipeName, false, receiverFinished);
+
+            receiver.startThread();
+
+            auto bytesWritten = pipe.write (&sendData, sizeof (sendData), 2000);
+
+            expect (receiverFinished.wait (4000));
+
+            expectEquals (bytesWritten, (int) sizeof (sendData));
+            expectEquals (receiver.result, (int) sizeof (receiver.recvData));
+            expectEquals (receiver.recvData, sendData);
+        }
+
+        beginTest ("Send message existing pipe");
+        {
+            WaitableEvent receiverFinished;
+            ReceiverThread receiver (pipeName, true, receiverFinished);
+
+            NamedPipe pipe;
+            expect (pipe.openExisting (pipeName));
+
+            receiver.startThread();
+
+            auto bytesWritten = pipe.write (&sendData, sizeof (sendData), 2000);
+
+            expect (receiverFinished.wait (4000));
+
+            expectEquals (bytesWritten, (int) sizeof (sendData));
+            expectEquals (receiver.result, (int) sizeof (receiver.recvData));
+            expectEquals (receiver.recvData, sendData);
+        }
+    }
+
+private:
+    //==============================================================================
+    struct NamedPipeThread   : public Thread
+    {
+        NamedPipeThread (const String& threadName, const String& pName,
+                         bool shouldCreatePipe, WaitableEvent& completed)
+            : Thread (threadName), pipeName (pName), workCompleted (completed)
+        {
+            if (shouldCreatePipe)
+                pipe.createNewPipe (pipeName);
+            else
+                pipe.openExisting (pipeName);
+        }
+
+        NamedPipe pipe;
+        const String& pipeName;
+        WaitableEvent& workCompleted;
+
+        int result = -2;
+    };
+
+    //==============================================================================
+    struct SenderThread   : public NamedPipeThread
+    {
+        SenderThread (const String& pName, bool shouldCreatePipe,
+                      WaitableEvent& completed, int sData)
+            : NamedPipeThread ("NamePipeSender", pName, shouldCreatePipe, completed),
+              sendData (sData)
+        {}
+
+        void run() override
+        {
+            result = pipe.write (&sendData, sizeof (sendData), 2000);
+            workCompleted.signal();
+        }
+
+        const int sendData;
+    };
+
+    //==============================================================================
+    struct ReceiverThread   : public NamedPipeThread
+    {
+        ReceiverThread (const String& pName, bool shouldCreatePipe,
+                        WaitableEvent& completed)
+            : NamedPipeThread ("NamePipeSender", pName, shouldCreatePipe, completed)
+        {}
+
+        void run() override
+        {
+            result = pipe.read (&recvData, sizeof (recvData), 2000);
+            workCompleted.signal();
+        }
+
+        int recvData = -2;
+    };
+};
+
+static NamedPipeTests namedPipeTests;
+
+#endif
+
 } // namespace juce