Infinite Loop in NamedPipe::read?


#1

I’ve been debugging a problem with a ConnectedChildProcess for hours now and I think it boils down to this:

In InterprocessConnection::readNextMessageInt, the pipeReceiveMessageTimeout is ignored and a value of -1 is always passed to the pipe’s read method:

bool InterprocessConnection::readNextMessageInt()
{
    uint32 messageHeader[2];
    const int bytes = socket != nullptr ? socket->read (messageHeader, sizeof (messageHeader), true)
                                        : pipe  ->read (messageHeader, sizeof (messageHeader), -1);

This means that in the read method, the first local variable timeoutEnd gets initialised to 0;
When the call to waitForInput happens, timeoutEnd is 0 so waitForInput will wait for the maxWaitingTime of 30ms. So far so good.

However, because the check a few lines above for hasExpired (timeoutEnd) uses the local timeoutEnd which is 0, the loop will run forever, never exiting:

int read (char* destBuffer, int maxBytesToRead, int timeOutMilliseconds)
{
    const uint32 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)
    {
        const int bytesThisTime = maxBytesToRead - bytesRead;
        const int numRead = (int) ::read (pipeIn, destBuffer, (size_t) bytesThisTime);

        if (numRead <= 0)
        {
            if (errno != EWOULDBLOCK || stopReadOperation || hasExpired (timeoutEnd))
                return -1;

            const int maxWaitingTime = 30;
            waitForInput (pipeIn, timeoutEnd == 0 ? maxWaitingTime
                                                  : jmin (maxWaitingTime,
                                                          (int) (timeoutEnd - Time::getMillisecondCounter())));
            continue;
        }

        bytesRead += numRead;
        destBuffer += numRead;
    }

    return bytesRead;
}

Now this might be the desired behaviour for this case and maybe it should block unless the thread is signalled to exit and the stopReadOperation flag set etc.

If this is the case, it would seem that in InterprocessConnection::readNextMessageInt the pipeReceiveMessageTimeout should be passed on so at least this has a chance to bail out?