Another way to hang NamedPipe::read on Windows

General JUCE discussion
1

I found another way to hang NamedPipe::read on Windows. Here’s what I did.

  • start process 1 (slave)
  • start process 2 (master)
  • master constructs juce::InterprocessConnection
  • slave constructs juce::InterprocessConnection
  • master calls createPipe
  • slave calls connectToPipe (so its read thread starts)
  • slave destroys juce::InterprocessConnection object

The ReadFile call in NamedPipe::read returned ERROR_PIPE_NOT_CONNECTED for which there isn’t an explicit check, so waitAgain gets set true and the loops goes around again. Eventually the call to stopThread in the InterprocessConnection::disconnect forcibly rips down the read thread.

What do you think about this change? I’m nervous that there are still some other conditions where we should bail out but I’m not familiar enough with how all the pieces work here to suggest anything else.

[code]diff --git a/src/native/windows/juce_win32_Files.cpp b/src/native/windows/juce_win32_Files.cpp
index 2bd1199…61ca566 100644
— a/src/native/windows/juce_win32_Files.cpp
+++ b/src/native/windows/juce_win32_Files.cpp
@@ -867,7 +867,7 @@ int NamedPipe::read (void* destBuffer, int maxBytesToRead, int timeOutMillisecon
intern->disconnectPipe();
}
}

  •        else if (lastError == ERROR_BROKEN_PIPE && intern->isPipe)

  •        else if (((lastError == ERROR_BROKEN_PIPE) || (lastError == ERROR_PIPE_NOT_CONNECTED)) && intern->isPipe)
       {
           intern->disconnectPipe();
       }

[/code]

2

Turns out this diff wasn’t enough to keep from hanging all the time. The read thread in question is for the slave side of the pipe…the one that calls InterprocessConnection::connectToPipe…which calls NamedPipe::openExisting, so the corresponding NamedPipeInternal::isPipe is false. This extra diff makes the hang go away, and even makes sense. If waitAgain gets set to true when isPipe is false, intern->connect returns true so ReadFile gets called again and presumably returns the same way it did before.

[code]diff --git a/src/native/windows/juce_win32_Files.cpp b/src/native/windows/juce_win32_Files.cpp
index 61ca566…e694e0d 100644
— a/src/native/windows/juce_win32_Files.cpp
+++ b/src/native/windows/juce_win32_Files.cpp
@@ -871,7 +871,7 @@ int NamedPipe::read (void* destBuffer, int maxBytesToRead, int timeOutMillisecon
{
intern->disconnectPipe();
}

  •        else

  •        else if (intern->isPipe)
       {
           waitAgain = true;
           Sleep (5);

[/code]

3

Good stuff, thanks! I’ve just been tidying up that class, I’ll double-check these things and make sure it handles them…