roadrunner-server / roadrunner

🤯 High-performance PHP application server, process manager written in Go and powered with plugins
https://docs.roadrunner.dev
MIT License
7.94k stars 414 forks source link

[🐛 BUG]: Failed to call goridge in the swoole coroutine #1951

Closed shellphy closed 5 months ago

shellphy commented 5 months ago

No duplicates 🥲.

What happened?

Since swoole has been used in our project, we did not directly use the main roadrunner program. Recently, we found your subproject goridge, and tried to call golang methods in swoole coroutines.

However, the issue submission function is not enabled in the goridge project, so issues are submitted here

My test code is as follows:

 \Co\run(function () {
        go(function () {
                $rpc = new Goridge\RPC\RPC(
                    Goridge\Relay::create('tcp://127.0.0.1:6001')
                );
                echo $rpc->call("App.Hi", "Antony");
        });

            go(function () {
                $rpc = new Goridge\RPC\RPC(
                    Goridge\Relay::create('tcp://127.0.0.1:6001')
                );
                echo $rpc->call("App.Hi", "Antony");
            });

            sleep(10);
        });

Both requests are sent to golang, and the result is that the first call is typed correctly, and the second call reports an error:

Hello, Antony! PHP Fatal error: Uncaught Spiral\Goridge\RPC\Exception\RPCException: Invalid RPC frame, sequence mismatch in /var/www/vendor/spiral/goridge/src/RPC/RPC.php:89

I'm using php7.4, spiral/goridge:3.2.0

Relevant log output

No response

rustatian commented 5 months ago

Hey @shellphy 👋 Yes, all related to the roadrunner-server issues are stored in 1 place - in the main RR project.

I'm not familiar with Swoole, but it looks like that it tries to send both request via the same RPC connection. I'm not sure what is \Co and what is go in that case.

All RPC connections (not exactly Goridge) should be consecutive, and I'm not sure, these go statements are not using the same connection.

shellphy commented 5 months ago

It's not the same connection. I tried to print out the rpc object:

\Co\run(function () {
            go(function () {
                $rpc = new Goridge\RPC\RPC(
                    Goridge\Relay::create('tcp://127.0.0.1:6001')
                );
                var_dump($rpc);
                echo $rpc->call("App.Hi", "Antony");
            });

            go(function () {
                $rpc = new Goridge\RPC\RPC(
                    Goridge\Relay::create('tcp://127.0.0.1:6001')
                );
                var_dump($rpc);
                echo $rpc->call("App.Hi", "Antony");
            });

            sleep(10);
        });

response:

object(Spiral\Goridge\RPC\RPC)#27 (3) {
  ["relay":"Spiral\Goridge\RPC\RPC":private]=>
  object(Spiral\Goridge\SocketRelay)#28 (4) {
    ["address":"Spiral\Goridge\SocketRelay":private]=>
    string(9) "127.0.0.1"
    ["port":"Spiral\Goridge\SocketRelay":private]=>
    int(6001)
    ["type":"Spiral\Goridge\SocketRelay":private]=>
    int(0)
    ["socket":"Spiral\Goridge\SocketRelay":private]=>
    NULL
  }
  ["codec":"Spiral\Goridge\RPC\RPC":private]=>
  object(Spiral\Goridge\RPC\Codec\JsonCodec)#29 (0) {
  }
  ["service":"Spiral\Goridge\RPC\RPC":private]=>
  NULL
}
object(Spiral\Goridge\RPC\RPC)#33 (3) {
  ["relay":"Spiral\Goridge\RPC\RPC":private]=>
  object(Spiral\Goridge\SocketRelay)#34 (4) {
    ["address":"Spiral\Goridge\SocketRelay":private]=>
    string(9) "127.0.0.1"
    ["port":"Spiral\Goridge\SocketRelay":private]=>
    int(6001)
    ["type":"Spiral\Goridge\SocketRelay":private]=>
    int(0)
    ["socket":"Spiral\Goridge\SocketRelay":private]=>
    NULL
  }
  ["codec":"Spiral\Goridge\RPC\RPC":private]=>
  object(Spiral\Goridge\RPC\Codec\JsonCodec)#35 (0) {
  }
  ["service":"Spiral\Goridge\RPC\RPC":private]=>
  NULL
}

Hello, Antony!
PHP Fatal error:  Uncaught Spiral\Goridge\RPC\Exception\RPCException: Invalid RPC frame, sequence mismatch in /var/www/vendor/spiral/goridge/src/RPC/RPC.php:89
rustatian commented 5 months ago

It would be nice if you'd try the following:

  1. Create two separate PHP files with the Goridge code (rpc) inside.
  2. Execute them at the same time.

I just want to check, if that is something Swooly related.

shellphy commented 5 months ago

I just debugging, in the vendor/spiral/goridge/SRC/RPC/RPC. PHP 88 lines of print log:

 public function call(string $method, $payload, $options = null)
    {
        $this->relay->send($this->packFrame($method, $payload));

        // wait for the frame confirmation
        $frame = $this->relay->waitFrame();

        if (\count($frame->options) !== 2) {
            throw new RPCException('Invalid RPC frame, options missing');
        }

        var_dump($frame);
        var_dump(self::$seq);
        if ($frame->options[0] !== self::$seq) {
            throw new RPCException('Invalid RPC frame, sequence mismatch');
        }

        self::$seq++;

        return $this->decodeResponse($frame, $options);
    }

The results are as follows:

object(Spiral\Goridge\Frame)#36 (5) {
["payload"]=>
string(24) "App.Hi"Hello, Antony! \n""
["options"]=>
array(2) {
[0]=>
int(1)
[1]=>
int(6)
}
["flags"]=>
int(8)
["byte10"]=>
int(0)
["byte11"]=>
int(0)
}
int(1)
Hello, Antony!
object(Spiral\Goridge\Frame)#26 (5) {
["payload"]=>
string(24) "App.Hi"Hello, Antony! \n""
["options"]=>
array(2) {
[0]=>
int(1)
[1]=>
int(6)
}
["flags"]=>
int(8)
["byte10"]=>
int(0)
["byte11"]=>
int(0)
}
int(2)

So I think the problem is that the $seq variable is static:

private static int $seq = 1;

Because in swoole's coroutine environment, static variables are shared because they are still the same process

rustatian commented 5 months ago

Nice, thanks 👍

rustatian commented 5 months ago

You may try to send a fix 😉

shellphy commented 5 months ago

I think it's possible to modify the handling here a little bit to make it compatible with swoole coroutines. It is better to put a patch on spiral/goridge:3.2.0 because our php environment is 7.4, thank you

rustatian commented 5 months ago

Yeah, sure, you may send a patch for review and link to this issue.

shellphy commented 5 months ago

Yeah, sure, you may send a patch for review and link to this issue.

I have just submitted a request, and I feel that there is no problem in my test. All testcases have passed, and there is no problem in the test in swoole

rustatian commented 5 months ago

Thank you @shellphy 👍 I see that in CI all tests are red. I guess, you need to install a Swoole extension in the CI test env.

rustatian commented 5 months ago

Fixed by: https://github.com/roadrunner-php/goridge/pull/30, thanks @shellphy 👍