dejanb / stomp-php

Stomp PHP Client
http://stomp.fusesource.org/documentation/php/
93 stars 46 forks source link

Client not handling broken connections while in read loop #11

Open ejsmont-artur opened 12 years ago

ejsmont-artur commented 12 years ago

Hi there

I have just noticed today that process gets stuck in read/poll loop if connection gets dropped or server dies.

This code does not seem to recognise that socket has been closed

$read = fread($this->_socket, $rb);

        if ($read === false) {
            $this->_reconnect();
            return $this->readFrame();
        }

I dont have patch for now as i could not find a way to check if file handler is still readable.

In case of server dying im getting back string "" from fread not FALSE. It was tested on php 5.3

Thanks

Art

marklr commented 12 years ago

This is an issue with PHP's wrapping of the actual C function, I think that the space check would be added along with !== false.

ejsmont-artur commented 12 years ago

well i have not got time to test it but if there was a way to distinguish between lost connection and no data then it would be all good. Do you think would feof work?

http://www.php.net/manual/en/function.feof.php

marklr commented 12 years ago

Wouldn't hurt to see whether checking for false and empty string does the job. Otherwise, we could use stream_get_meta_data and check for 'unread_bytes', etc.

ejsmont-artur commented 12 years ago

Well, maybe, I thought on read timeout you will get empty string as well but maybe it would be a null? :)

I wanted to set read timeout to some 10s and also have outer loop with a check if i exceeded count of messages or time to live and end process to garbage collect it. PHP was never fully reliable as an infinite-loop tool, but mabe nowadays its ok and im just too cautious :)

lirenzhu commented 11 years ago

The issue here is with hasFrameRead method. As a direct result of the first if statement ($has_frame_to_read !== false), it will always consider a timed out connection as "having a frame to read," so to speak.

bathizte commented 11 years ago

Testing feof inside hasFrameToRead resolves the issue :

if(feof($this->_socket)){
    require_once 'Stomp/Exception.php';
    throw new StompException('Disconnected');
}