ratchetphp / Ratchet

Asynchronous WebSocket server
http://socketo.me
MIT License
6.24k stars 727 forks source link

PHP Fatal error: Call to a member function handshake() on a non-object #238

Closed abienvenu closed 8 years ago

abienvenu commented 9 years ago

My 0.3.2 Ratchet server has been running smoothly for months on PHP 5.5.10, with hundreds of people connected to it. And last night, it crashed :

[02-Oct-2014 03:32:55 Europe/Paris] PHP Notice:  Undefined property: stdClass::$version in /symfony/vendor/cboden/ratchet/src/Ratchet/WebSocket/WsServer.php on line 114
[02-Oct-2014 03:32:55 Europe/Paris] PHP Fatal error:  Call to a member function handshake() on a non-object in /symfony/vendor/cboden/ratchet/src/Ratchet/WebSocket/WsServer.php on line 114

It happened in the middle of the night, so the client was probably a broken bot or some hacker.

Just in case, here is my main loop :

        $server = new Server();
        $wsServer = new WsServer($server);
        $httpServer = new HttpServer($wsServer);
        $loop = IoServer::factory($httpServer, $port);
        $loop->run();

And here is my Server class :

use Ratchet\MessageComponentInterface;
use Ratchet\ConnectionInterface;

class Server implements MessageComponentInterface
{
    protected $clients;

    public function __construct()
    {
        $this->clients   = new \SplObjectStorage;
    }

    public function onOpen(ConnectionInterface $connection)
    {
        $this->clients->attach($connection);
    }

    public function onMessage(ConnectionInterface $connection, $message)
    {
        // Do stuff

        if ($broadcastOk) {
            foreach ($this->clients as $client) {
                $client->send(json_encode($data));
            }
        }
        else {
            $connection->close();
        }
    }

    public function onClose(ConnectionInterface $connection)
    {
        $this->clients->detach($connection);
    }

    public function onError(ConnectionInterface $connection, \Exception $e)
    {
        $this->clients->detach($connection);
        $connection->close();
    }
}

Any ideas about the cause of this crash ?

cboden commented 9 years ago

I think this is the culprit.

This dates back to the Hixie76 protocol. The header will be buffered received but not the body yet. If the 8 byte body is chunked in two or more pieces I think this could trigger the error your having.

I'll do a bit more testing and get back to you.

cboden commented 9 years ago

Nope, that wasn't it. I just wrote a new unit test for that scenario and it passed. $request would be valid because HttpServer won't pass a connection to WsServer until it has all the headers. As well Hixie76 detection happens by checking a specific header key rather than the body.

abienvenu commented 9 years ago

Thanks for the investigation. Until we find a way to reproduce and fix this bug, we use monit to watch the server. I will get back to you if I can get more information.

cboden commented 9 years ago

Unrelated, but I think you may want to remove the detach call in onError. Calling close will eventually make it bubble up to your onClose call where it will detach again. For some reason PHP throws an unrecoverable error when you call detach on an SplObjectStorage when the object isn't contained in it.

abienvenu commented 9 years ago

Thank you for the tip. I added this detach because I had a memory leak. But maybe the leak was somewhere else in my code. I trust you, I will remove it and give it a try next week.

abienvenu commented 9 years ago

FYI, I removed the detach, and I get no leak.

I will keep you posted if I get more "PHP Fatal error".

mart1 commented 9 years ago

I have your heisenbug with my code:

PHP Notice: Undefined property: stdClass::$version in /root/MyApp/vendor/cboden/ratchet/src/Ratchet/WebSocket/WsServer.php on line 109 PHP Fatal error: Call to a member function handshake() on a non-object in /root/MyApp/vendor/cboden/ratchet/src/Ratchet/WebSocket/WsServer.php on line 109


namespace Crm;
use Ratchet\MessageComponentInterface;
use Ratchet\ConnectionInterface;

class Dispatch implements MessageComponentInterface {
    protected $clients;
    protected $pamiConn;
    public function __construct() {
        $this->clients = array();
        $this->pamiConn = null;
    }

    public function onOpen(ConnectionInterface $conn) {
        // Store the new connection to send messages to later
        //$this->clients->attach ( $conn );
        echo "New connection! ({$conn->resourceId})\n";
    }

    public function onMessage(ConnectionInterface $from, $msg) {
        echo "onMessage\n";
        print_r($msg);
        echo "\n";
        $json = json_decode ( $msg );
        if (isset ( $json->sessid )) {
            $sessid = $json->sessid;
            echo "$sessid\n";
            // code special pour le PAMI
            if ($sessid == "P@mI") {
                echo "PAMI connected\n";
                $this->pamiConn = $from;
                $this->clients["PAMI"]=array("from"=>$from);
            } else {
                // on ne recoit que la session
                // on recupere le device pour ce sessid
                $opts = array ('http' => array ('method' => 'POST', 'header' => 'Content-type: application/x-www-form-urlencoded', 'content' =>  http_build_query(array ('session' => $sessid ) ) ) );
                $context = stream_context_create ( $opts );
                ///$r = new HttpRequest ( 'xx', HttpRequest::METH_POST );

                //$r->addPostFields ( );

                try {
                    $res = file_get_contents('xx', false, $context);
                    echo "$res\n";
                    $json = json_decode ( $res );
                    if (isset ( $json->result->message ) && $json->result->message == 'OK') {
                        $mac = json_decode ( $res )->result->device;

                        if(array_key_exists($mac,$this->clients)){
                            echo $mac . " already connected (écrasement)\n";
                            $this->clients[$mac] = array ("sessid" => $sessid, "from" => $from ) ;
                        }else{
                            $this->clients[$mac] = array ("sessid" => $sessid, "from" => $from ) ;
                            echo $mac . " connected\n";
                        }

                    }
                    if(!is_null($this->pamiConn)){
                        // TODO:déclaration du téléphone auprès de PAMI
                        $this->pamiConn->send (json_encode(array("action"=>"add","mac"=>$mac)));
                    }else
                    {
                        echo "PAMI n'est pas connecte\n";
                    }
                } catch ( HttpException $ex ) {
                    echo $ex;
                }
            }

        } else if (! is_null ( $this->pamiConn ) && $this->pamiConn == $from) { // pas de sessid alors c'est PAMI
            $json = json_decode ( $msg );
            //$json = json_decode(json_encode ( array ("number" => "4545", "mac" => "5454" ) ));
            // TODO: un message de PAMI a relayer a un telephone
            if (isset ( $json->mac ) && isset ( $json->number )) {

                if(isset($this->clients [$json->mac])){

                    $this->clients [$json->mac] ["from"]->send ('{"result":{"message":"OK","callernumber":"'.$json->number.'"}}');

                }else{
                    echo "Telephone non enregistre";
                }
            } else {
                echo "Données manquantes";
            }
        } else {
            echo "Demande inconnue";
        }

        // on récupere le device par rapport au nic

    }
    public function close(){
        foreach($this->clients as $data){
            $data["from"]->close();
        }
        }

    public function onClose(ConnectionInterface $conn) {
        // The connection is closed, remove it, as we can no longer send it messages

        foreach($this->clients as $mac=>$data){
            if($data["from"]==$conn){
                echo  $mac ." disconnected\n";
                unset($this->clients[ $mac ]);

                if(!is_null($this->pamiConn))$this->pamiConn->send (json_encode(array("action"=>"remove","mac"=>$mac)));
                break;
            }
        }

        echo "Connection {$conn->resourceId} has disconnected\n";
    }

    public function onError(ConnectionInterface $conn,\Exception $e) {
        echo "An error has occurred: {$e->getMessage()}\n";

        $conn->close ();
    }
}
cboden commented 9 years ago

@mart1 Are you able to provide steps to reproduce the problem? What browser were you using when the error happened?

@abienvenu @mart1 @RynnHeldeD I made PR #337. Can you guys test that out and see if the error continues? I think I see how the error has occurred but I don't know what is causing it to happen. My best guess is a browser is doing something it shouldn't be doing.

RynnHeldeD commented 9 years ago

@cboden Thanks for the fix, I'll try it as soon as I'll understand how to implement it. As I installed Ratchet in my project using Composer, I suppose I just have to replicate the changes you made in your PR #337, then test my project again until the problem happens again (or never if it really fixes it) ?

cboden commented 9 years ago

@RynnHeldeD Update your composer file from Ratchet's version to look like this (and run composer update):

"cboden/ratchet": "dev-handshake-fix"

RynnHeldeD commented 9 years ago

Ok, done ! I'll tell you if there is some news about this strange bug.

2015-07-15 22:38 GMT+02:00 Chris Boden notifications@github.com:

@RynnHeldeD https://github.com/RynnHeldeD Update your composer file from Ratchet's version to look like this (and run composer update):

"cboden/ratchet": "dev-handshake-fix"

— Reply to this email directly or view it on GitHub https://github.com/ratchetphp/Ratchet/issues/238#issuecomment-121740048.

cboden commented 8 years ago

Hopefully fixed in #337. Please report if this issue still persists.