amphp / socket

Non-blocking socket and TLS functionality for PHP based on Amp.
https://amphp.org/socket
MIT License
237 stars 38 forks source link

Socket is not really non blocking or async #115

Closed Tofandel closed 3 weeks ago

Tofandel commented 3 weeks ago

It seems the async method does not do what it advertises to do

Here is a simple server taken from the readme with a sleep and logs added

#!/usr/bin/env php
<?php // basic (and dumb) HTTP server

require __DIR__ . '/vendor/autoload.php';

// This is a very simple HTTP server that just prints a message to each client that connects.
// It doesn't check whether the client sent an HTTP request.

// You might notice that your browser opens several connections instead of just one,
// even when only making one request.

use Amp\Socket;
use function Amp\async;

$server = Socket\listen('127.0.0.1:0');

echo 'Listening for new connections on ' . $server->getAddress() . ' ...' . PHP_EOL;
echo 'Open your browser and visit http://' . $server->getAddress() . '/' . PHP_EOL;

while ($socket = $server->accept()) {
    async(function () use ($socket) {
        $address = $socket->getRemoteAddress();
        $ip = $address->toString();

        echo "Accepted connection from {$address}." . PHP_EOL;

        // Run heavy task here
        sleep(10);
        $body = "Hey, your IP is {$ip}.";
        $bodyLength = \strlen($body);

        $socket->write("HTTP/1.1 200 OK\r\nConnection: close\r\nContent-Length: {$bodyLength}\r\n\r\n{$body}");
        $socket->end();

        echo "Request from {$address} processed" . PHP_EOL;
    });
}

Running the socket and then connecting to the server with multiple clients results in the later clients being blocked sequentially until each client finishes image

Am I missing something? Do I need to use amphp/parallel (which doesn't seem to work well with sockets which are non serializable)

Why doesn't it use parallel under the hood for accepting connections already? It cannot be advertised as "non-blocking" as it is

kelunik commented 3 weeks ago

The issue is that your sleep() is blocking. Use Amp\delay() instead.

Tofandel commented 3 weeks ago

That's not the issue no, if anything inside any other thing takes more than a seconds to run then it's a problem because execution is still sequential and anything inside async still delays accepting connections, so then it's not really async and it's a misleading name..

This is an example to just simulate an expensive operation. Which would effectively block the server. The only answer to this problem (parallelism) in php seems to be creating fork or child processes to have the load running in different processes so that the main loop can handle connections without ever blocking

I tried to do some forking with the amphp/socket implementation and it just doesn't work, the socket connections do not handle being forked like native php sockets do

Bilge commented 3 weeks ago

CTO of having no clue what you're talking about.

Tofandel commented 3 weeks ago

Yeah sure whatever, thanks for the hostility. Not going to bother with this then

Bilge commented 3 weeks ago

If you want async, you use async end-to-end. The moment you stick anything blocking inbetween, it's not async. That's why there's an Amp library for every major protocol.

bwoebi commented 3 weeks ago

@Bilge May you please be a bit nicer...

@Tofandel As Bilge said though, if you want to parallelize blocking operations you need to fork or use threading (i.e. amp/parallel covers that). ... Or use PHP code which is not using blocking operations. Using async() doesn't magically make code non-blocking, the functions used within to be non-blocking too.

Tofandel commented 3 weeks ago

@bwoebi Thanks, yes this is what I thought as well, but forking just doesn't work here, I tried the same way I usually fork a process upon receiving a connection on a php socket (which works reliably) but with amphp sockets if I close the socket in the parent process, it closes it in the child process (becomes unwrittable and unreadable). And if I don't close it in the parent process, the receive operation just hangs forever not getting any data

I would rather fork because then the state of the program is copied and there is a lot of stuff I don't need to worry about

With amp/parallel, the sockets cannot be serialized so while an option to run the long task, I just need to receive the full data beforehand (or have to deal with writing a 2 way proxy between the socket and the channel) . I would also need an option to run a bootstrap on the workers, to run some setup only once when the workers are created so that the tasks dont need to do that individually and they can run a bit faster, but I didn't see that option

trowski commented 3 weeks ago

amphp/parallel does allow you to run a bootstrap file in a worker when it is first launched. See Amp\Parallel\Worker\ContextWorkerFactory, which has a $bootstrapPath as one of the constructor arguments. You need to provide this worker factory to the worker pool.

You otherwise can use a Context directly in amphp/parallel instead of the workers. Workers are somewhat opinionated, whereas the Context API lets you write your own custom PHP script and run it as a child process (or thread if you happen to install ext-parallel).

Sockets actually can be serialized and set to a child worker, but that requires ext-sockets, so it lives in another library, amphp/cluster. See the readme section on transferring sockets