Skip to content

Emulate non-blocking pipes in proc_open() on Windows #5864

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Emulate non-blocking pipes in proc_open() on Windows #5864

wants to merge 2 commits into from

Conversation

kooldev
Copy link
Contributor

@kooldev kooldev commented Jul 15, 2020

This PR adds a new option threaded_pipes to other_options in proc_open(). This feature (like all the other options) is exclusive to Windows. Threaded pipes provide non-blocking streams in situations where the solution implemented in #5777 is not supported by the spawned child process (for example node.js fails to do so). The downside of this approach is the need to spawn a thread for each non-closed process pipe. In addition to that we need an additional pair of sockets for each pipe. The need to tunnel data through threads involves additional system calls (ReadFile(), WriteFile(), WSARecv(), WSASend()) that add (a little) latency to process IO operations. We do however end up with a solution that can avoid all issues related to child process pipe blocking on Windows reliably.

Threaded pipes are backed by a Windows thread, an anonymous pipe (the parent end of a process pipe) and a socket pair. In read mode (STDOUT / STDERR) the thread will do blocking reads from the process pipe and write the received data to one end of the socket pair. The other end of the socket pair is exposed to PHP userland. It can be used to read data from the process pipe, supports non-blocking mode and can be polled for events by stream_select(). Write mode (STDIN) works in the same way, the thread reads data from the socket pair and writes it to the process pipe. The other end of the socket pair is exposed to userland and has full support for non-blocking / select.

This little example shows how to use threaded pipes. Note that I used the exact same example in #5777 and in the case of ping we could use socket instead of pipe just fine (it would be faster and use fewer system resources). Spawning a node.js proccess works just fine with threaded pipes whereas it would fail with just sockets.

$spec = [
    0 => ['null'],
    1 => ['pipe', 'wb'],
    2 => ['pipe', 'wb']
];

$pipes = null;
$cmd = ['ping', 'github.com'];

$proc = proc_open($cmd, $spec, $pipes, null, null, [
    'threaded_pipes' => true
]);

foreach ($pipes as $pipe) {
    var_dump(stream_set_blocking($pipe, false));
}
print_r($pipes);
$buffers = array_fill_keys(array_keys($pipes), '');

while ($pipes) {
    $r = $pipes;
    $w = null;
    $e = null;

    if (!stream_select($r, $w, $e, null, 0)) {
        throw new \Error("Select failed");
    }
    
    foreach ($r as $i => $pipe) {
        if (!is_resource($pipe) || feof($pipe)) {
            $line = trim($buffers[$i]);
            
            if ($line !== '') {
                echo "PIPE {$i} << {$line}\n";
            }
            
            echo "DROP pipe {$i}\n";
            unset($pipes[$i]);
            continue;
        }
        
        $chunk = @fread($pipe, 8192);
        
        if ($chunk === false) {
            throw new \Error("Failed to read: " . (error_get_last()['message'] ?? 'N/A'));
        }
        
        $buffers[$i] .= $chunk;
        
        if (false !== strpos($buffers[$i], "\n")) {
            $lines = array_map('trim', explode("\n", $buffers[$i]));
            $buffers[$i] = array_pop($lines);
            
            foreach ($lines as $line) {
                echo "PIPE {$i} << {$line}\n";
            }
        }
    }
}

It is possible to change the threading model and use a single thread to power all pipes but it will be a lot more complicated to implement and only be beneficial if a lot of processes / pipes are alive at the same time. The only model that can achieve this on Windows is IOCP. Using it would require us to manually create a pair of named pipes and enable overlapped mode / IOCP for the parent end. It would also require the (internal) end of the socket pair to use overlapped mode and the same IOCP. The thread processing all of this would need to call GetQueuedCompletionStatus() and handle results and pipe state. While this model makes sense when all your IO is async / non-blocking I think it will not offer much benefit for the proc_open() use case.

The basic idea (and maybe even implementation) could also be reused to provide non-blocking access to PHP's STDIN / STDOUT / STDERR when PHP is run from the cli. This is just an idea for a possible addition, none of this is implemented currently.

@@ -0,0 +1,69 @@
--TEST--
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is missing a --SKIPIF-- section.

We generally use PHP_OS_FAMILY !== 'Windows' or this in other tests

Copy link
Contributor Author

@kooldev kooldev Jul 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left out SKIPIF because I want the test to verify that pipes behave in exactly the same way on Linux and Windows systems when threads are used on Windows. The threaded_pipes option has to be ignored for all other builds.

@nikic nikic requested a review from cmb69 July 16, 2020 08:06
@kooldev kooldev marked this pull request as draft July 16, 2020 09:16
@kooldev
Copy link
Contributor Author

kooldev commented Jul 16, 2020

@nikic @cmb69 I forgot to flag the PR as a draft. It needs better error handling (errors in threads need to be propagated to the stream in order to trigger a warning and fail stream operations) and socket / temp buffer sizes will likely need to be adjusted. I created the PR to get some feedback if this feature is something that you would like to see implemented in PHP before I put in more work.

Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! That's certainly a fascinating solution for a real problem (although I had hoped that would have been completely solved with PR #5777). The "obvious" solution for the issue of blocking pipes would be to use overlapped IO for these in the first place, but that would be way more intrusive, so this solution might be preferable. I'd need more time to come to a well-informed conclusion.

Anyway, @DaveRandom may have some thought about this.

@kooldev
Copy link
Contributor Author

kooldev commented Jul 17, 2020

@cmb69 Thanks for lloking into this. I know that there are other ways to implement this. Let me provide some background why I ended up implementing it the way I did:

The ultimate goal is to expose process pipes as PHP streams to userland. The solution currently provided by proc_open() works fine if only a single pipe is needed and there is no need for it to be non-blocking. In cases where more than one pipe is needed we need a way to alternate reading from / writing to pipes. This requires pipes to support non-blocking mode and a way to use them with a select / poll call (an alternative is to do non-blocking read / write in a loop with a (u)sleep, but this is basically busy waiting and thus should be avoided). The only select call that PHP provides is stream_select() and it works for sockets on any OS. While it supports pipes on Linux / Mac it cannot do that on Windows.

[ PHP stream ] <==> [ socket pair ] <==> [ IO thread ] <==> [ process pipe ]

This is the way the "threaded pipe" (as implemented in this PR) wraps a Windows pipe to make it fit into PHP's concecpt of non-blocking IO. The socket pair acts as a proxy between a PHP stream and the process pipes. The stream supports non-blocking mode and event notification because it wraps one end of the socket pair. A nice property of that design is that there is no need to explicitly synchronize threads because it comes for free with socket IO operations. The obvious downside is the need for a socket pair and a thread for each process pipe and the additional IO performed by the thread that adds IO calls and a little latency.

[ PHP stream 1 ] <==> [ socket pair 1 ] <==> [ IOCP thread ] <==> [ process pipe 1 ]
[ PHP stream 2 ] <==> [ socket pair 2 ] <====^  ^       ^  ^====> [ process pipe 2 ]
[ PHP stream 3 ] <==> [ socket pair 3 ] <=======|       |=======> [ process pipe 3 ]

This illustrates an alternative design that I considered. It involves a single IOCP thread that multiplexes IO calls across all process pipes. While it does save resources by using only a single thread it adds a lot of complexity to the implementation. It would require us to manually create a named pipe pair to pass to the child process (to have the parent end of the pipe support overlapped mode). It would also require one end of the socket pair to support overlapped IO (which is already the case). In addition to that the IOCP thread has to keep state for each socket-pipe pair to alternate between read and write. In addition to that is requires synchronized access to the IOCP thread to manage active pipes. And finally closing a PHP stream requires cancellation of pending IO operations and another sync with the IOCP thread to remove the stream from the overlapped IO set.

I wanted to keep things simple and went with the first design for the PR. It is meant to be used as fallback solution in situations where #5777 fails to deliver. I do not know of any way to make this work without putting the socket pair between PHP userland and the pipe. Even with IOCP there is no direct way to integrate a process pipe with stream_select() on Windows because IOCP is a completion-based model and does not play nice with the readiness-based model of select.

If anyone can think of an alternative (or even better way) to achieve this I would be very interested to know. I hope this was not too much information. I just wanted to write it down for reference. Maybe it will be helpful to anyone (my future self included). ;-)

Copy link
Member

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the feature, but can't meaningfully review this as I'm not familiar with the relevant Windows APIs, or even their I/O model.

{
threaded_pipe *data = (threaded_pipe *) stream->abstract;

if (threaded_error_check(data, "Pipe write failed: [%d] %s")) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer passing in only "Pipe write failed" and let the rest of the message be constructed by threaded_error_check. Separating format string from arguments makes me uncomfortable...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also don't really get why the error check is performed before the operation?

@@ -420,7 +420,7 @@ SECURITY_ATTRIBUTES php_proc_open_security = {
.bInheritHandle = TRUE
};

# define pipe(pair) (CreatePipe(&pair[0], &pair[1], &php_proc_open_security, 0) ? 0 : -1)
# define pipe(pair) (CreatePipe(&pair[0], &pair[1], &php_proc_open_security, 8192) ? 0 : -1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the magic 8192 value here?

php_socket_t sock; /* Internal end of the proxy socket pair. */
php_stream *stream; /* Wraps the exposed end of the socket pair. */
const char *mode; /* Pipe open mode. */
int closed; /* Set to 1 on stream close. */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
int closed; /* Set to 1 on stream close. */
bool closed; /* Set to 1 on stream close. */

/* Interrupt IO calls in thread and wait with timeout to prevent race conditions. */
do {
CancelSynchronousIo(data->thread);
} while (WAIT_TIMEOUT == WaitForSingleObject(data->thread, 5));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks a bit fishy...

@kooldev
Copy link
Contributor Author

kooldev commented Jul 31, 2020

Status Update: I did a lot more testing and while this soultion works for pipes that the child process writes to (STDOUT, STDERR) it fails to work with STDIN in the correct way. My assumption was that I could use the socket pair as a data buffer sitting between userland and the pipe. The important thing is to keep the socket buffer size small to ensure that all buffered data can be written to the pipe without blocking. The only way to do this is to set SO_SNDBUF to 0 and adjust SO_RCVBUF to the desired size. Windows ignores all of that for AF_INET (TCP) sockets on localhost (loopback) and does it's own thing. All data is sent into the socket and the socket is always reported to be writable. The buffer will fill up and it might block forever when fclose() is called, because all buffered data has to be written to the pipe.

I also tried with AF_UNIX sockets on my Windows 10 machine and it works with the correct buffer settings in the way I expected. However unix sockets on Windows (at least on my computer) will not use non-blocking mode. All calls to send() / WSASend() will always block until all data is written. Even explicitly activating non-blocking mode before any call does not change this.

Due to the these issues I lean towards closing this PR because it does not achieve what I intended it to do in a satisfactory way. There seem to be no workarounds for the TCP case and finding any resources related to using AF_UNIX on Windows is close to impossible...

@cmb69
Copy link
Member

cmb69 commented Jul 31, 2020

However unix sockets on Windows (at least on my computer) will not use non-blocking mode.

Bummer! (guess it wouldn't make sense to add general AF_UNIX support then)

So for solving the blocking pipes issue on Windows, we're likely back to using overlapped IO in the streams layer – fiddly, and likely too late for 8.0.

Anyhow, thanks for working on this – highly appreciated! :)

@kooldev
Copy link
Contributor Author

kooldev commented Jul 31, 2020

@cmb69 Do you know of a way to make overlapped IO work with PHP streams? I am (at least a bit) familiar with IOCP but I do not see a way to use it in non-blocking streams. AFAIK overlapped IO just schedules operations that send completion messages to a queue whereas PHP would need a way to know if an operation can be performed without blocking (to be used with stream_select()).

@cmb69
Copy link
Member

cmb69 commented Jul 31, 2020

I thought of not using completion routines, but rather WaitForMultipleObjects() for select()ing (basically, this vs. that). I'm not sure, though, if that would work for our stream layer. And quite likely, it wouldn't scale up that well, but still.

@kooldev
Copy link
Contributor Author

kooldev commented Aug 23, 2020

I am closing this PR due to problems with both TCP and UNIX sockets on Windows that (seemingly) cannot be worked around. Using overlapped IO / IOCP for process pipes is the way to go on Windows, but I cannot see how it could fit PHP's concept of non-blocking streams...

@kooldev kooldev closed this Aug 23, 2020
@mvorisek
Copy link
Contributor

So for solving the blocking pipes issue on Windows, we're likely back to using overlapped IO in the streams layer – fiddly, and likely too late for 8.0.

@cmb69 @kooldev can the blocking issues be somehow solved for upcoming PHP 8.1?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants