Keepalive processing needs two timers

[jpwsutton]

migrated from Bugzilla #442692 status NEW severity normal in component MQTT-JS for 1.1 Reported in version 1.0 on platform PC Assigned to: Tang Zi Han

On 2014-08-27 07:13:16 -0400, Ian Craggs wrote:

I realized yesterday that the keepalive processing in the C client was not correct. In discussion with Al we realized that two timers are needed, one for keeping track of the last packet sent, the other to keep track of the last packet received.

This is because they are used for two different purposes. The last packet sent timer is used to stop the server closing the session. A ping must be sent within the keepalive interval if no other packet has been sent to the server, to notify it that the client application is still alive.

The last packet received timer is used to determine that the TCP connection is still active. If you pull the network cable out of a machine, TCP writes will still work until the TCP buffer is full. Even then, you only get a buffer full notification (on Linux at least), not a network error. Only when the TCP keepalive timeout kicks in would the client be notified that the network had failed. So, if packet has not been received from the server within a reasonable amount of time (the keepalive time is a good value), then a ping should also be sent. If a response is not received from the server, then the TCP connection can be assumed to have failed, and the "connection lost" processing should be invoked.

This needs to be checked for the Javascript client.

[vonnieda]

FWIW, I ran into this issue in the embedded-c client and fixed it by adding a second timer: https://github.com/eclipse/paho.mqtt.embedded-c/issues/122#issuecomment-406683985

[icraggs]

I believe that a correct implementation is in the embedded C++ client library. I'm updating the embedded C version to match. It's close in the develop branch.

template<class Network, class Timer, int MAX_MQTT_PACKET_SIZE, int b> int MQTT::Client<Network, Timer, MAX_MQTT_PACKET_SIZE, b>::keepalive() { int rc = SUCCESS; static Timer ping_sent;

if (keepAliveInterval == 0)
    goto exit;

if (ping_outstanding)
{
    if (ping_sent.expired())
    {
        rc = FAILURE; // session failure
        #if defined(MQTT_DEBUG)
            DEBUG("PINGRESP not received in keepalive interval\r\n");
        #endif
    }
}
else if (last_sent.expired() || last_received.expired())
{
    Timer timer(1000);
    int len = MQTTSerialize_pingreq(sendbuf, MAX_MQTT_PACKET_SIZE);
    if (len > 0 && (rc = sendPacket(len, timer)) == SUCCESS) // send the ping packet
    {
        ping_outstanding = true;
        ping_sent.countdown(this->keepAliveInterval);
    }
}

exit: return rc; }