Call to a member function send() on null

[tomzx]

hoaproject/Websocket 2.15.08.05

I have setup a small websocket server which only dispatches message to all other connected clients. I use client code available @ http://hoa-project.net/En/Awecode/Websocket.html to connect to the server with ab: ab -c 50 -n 500 http://localhost/client-push.php.

However, it appears that as soon as I do that, the websocket server crashes with the following error:

PHP Fatal error: Call to a member function send() on null in /site/vendor/hoa/websocket/Connection.php on line 508 PHP Stack trace: PHP 1. {main}() /site/API/X/Server.php:0 PHP 2. NAMESPACE\Server::run() /site/API/X/Server.php:11 PHP 3. Hoa\Socket\Connection\Handler->run() /site/src/Z/Y/X/Server.php:16 PHP 4. Hoa\Websocket\Connection->_run() /site/vendor/hoa/socket/Connection/Handler.php:186 PHP 5. Hoa\Core\Event\Listener->fire() /site/vendor/hoa/websocket/Connection.php:310 PHP 6. Hoa\Core\Consistency\Xcallable->__invoke() /site/vendor/hoa/core/Event.php:524 PHP 7. call_user_func_array:{/site/vendor/hoa/core/Consistency.php:762}() /site/vendor/hoa/core/Consistency.php:762 PHP 8. NAMESPACE\Server::NAMESPACE{closure}() /site/vendor/hoa/core/Consistency.php:762 PHP 9. Hoa\Socket\Connection\Handler->broadcast() /site/src/Z/Y/X/Server.php:14 PHP 10. call_user_func_array:{/site/vendor/hoa/socket/Connection/Handler.php:288}() /site/vendor/hoa/socket/Connection/Handler.php:288 PHP 11. Hoa\Socket\Connection\Handler->broadcastIf() /site/vendor/hoa/socket/Connection/Handler.php:288 PHP 12. call_user_func_array:{/site/vendor/hoa/socket/Connection/Handler.php:323}() /site/vendor/hoa/socket/Connection/Handler.php:323 PHP 13. Hoa\Websocket\Connection->send() /site/vendor/hoa/socket/Connection/Handler.php:323 PHP 14. Hoa\Socket\Connection\Handler->Hoa\Socket\Connection{closure}() /site/vendor/hoa/websocket/Connection.php:534 PHP 15. call_user_func_array:{/site/vendor/hoa/socket/Connection/Handler.php:257}() /site/vendor/hoa/socket/Connection/Handler.php:257 PHP 16. Hoa\Websocket\Connection->Hoa\Websocket{closure}() /site/vendor/hoa/socket/Connection/Handler.php:257

Note that if I run ab without concurrency, the server does not crash.

Thanks by the way for this awesome library :)

[Hywan]

Hello :-),

Why do we call send without having done the handshake? Do you have a clue?

[Hywan]

Does https://github.com/hoaproject/Socket/pull/32 solve the issue?

[tomzx]

@Hywan Good question. I haven't dived deep enough in the code to make any suggestion as to why it is called other than the fact that it appears that the resource is part of the $connection->select() iterator result (maybe it should not be until the hanshake is complete?).

As you pointed out, maybe hoaproject/Socket#32 could have solved this issue on its own. When I first started to debug these two issues, this was the first one I fixed.

So I decided to try if removing this PR change would not crash. Sadly it does not appear to be the case and this change is still required (as of now).

[Hywan]

I am assigning @Metalaka to this serie of PR and issues since I have a lot of work. Please @Metalaka, tell me if you agree.

[Metalaka]

Hi, You're right https://github.com/hoaproject/Socket/pull/32 don't solve this issue, since the handshake is done inside Hoa\Socket\Connection\Handler::_run.

The simple use case is: (tell me if I'm wrong)

1. A client connect to the server S
2. B client connect to the server S
3. handshake A-S
4. A send Hello message
5. server broadcast A's message (to B here) 6. Exception: B connection is not yet ready (7). handshake B-S …

The issue seem to be related to the socket layer, not only for websocket. So why not adding your check inside Hoa/Socket/Connection/Handler.php#L246 before the call to the abstract method _send ? So both socket and websocket implementations will be fixed.

[tomzx]

@Metalaka Your use case seems to explain correctly the issue at hand.

However, I'm unsure of what you mean by

adding your check inside Hoa/Socket/Connection/Handler.php#L246

? Are you suggesting to use the exact same code? If so, this would require to check whether the connection is a WebSocket connection since the getHandshake method is only available on a WebSocket connection.

[Metalaka]

You're totally right, there is no handshake on Hoa\Socket.

So why not before Hoa/Websocket/Connection.php#L528? It's before in the call stack, thus we avoid some useless things.

[tomzx]

@Metalaka That may be more appropriate indeed. In my case, I simply fixed it the closest to the issue.

I'll give this suggestion a try later on tonight and I'll update this comment.

[Hywan]

What the status of this issue?

[Metalaka]

@Hywan sorry for the delay

Actually the issue can be fixed by checking if the handshake is done before sending data to this node.

But what about adding Hoa\Socket\Node::isReadyToCommunicate() method ? (probably with an other name)

---

Hoa\Websocket\Connection::send()'s patch:

    public function send(
    …
    ) {
        if (null === $node || !$node->getHandshake()) {
            return;
        }

        $send = parent::send($message, $node);

note the patch isn't on the same place as this PR.

[Hywan]

@Metalaka No worries :-). I just send pings ;-). I would like to start with an integration test that fails, to be able to reproduce. And then, to have a fix. Is the scenario in https://github.com/hoaproject/Websocket/pull/57#issuecomment-159407888 correct?

[Metalaka]

@Hywan Yes I think this scenario is correct.

[Metalaka]

@Hywan please forget the patch in my previous commit, it break the client side code.

When a client send a message to a server the $node variable is null to indicate a default value. This value is set after Hoa\Websocket\Connection::send() call, then the best place to put the patch is where @tomzx did (in Hoa\Websocket\Connection::_send() ).

So :+1: for this PR

[Hywan]

@Metalaka So we can merge this PR but not #60?

[Metalaka]

@Hywan #60 is about an other issue, it's the second part of my fix for https://github.com/hoaproject/Socket/pull/32

So, both PR can be merge.

[Hywan]

Great.

[Hywan]

Merged in https://github.com/hoaproject/Websocket/commit/1a8ef13e8493587cb480a9e1f0c61449404ff909!

[Hywan]

Thank you very much for your work :-].

[Hywan]

@tomzx I would have to release a new snapshot of this library but I would like to wait on #60 before. If any issue with this, raise your voice!