From 95f5191cd3b76eee7660a764eecd9a26b8db1f3d Mon Sep 17 00:00:00 2001 From: Datong Sun Date: Mon, 29 Jun 2020 21:35:12 -0700 Subject: [PATCH] Fixed an regression that MQTT client timer is disarmed prematurely when connecting to server. Inside af426d0315f4c18d13468acf131f7f91a436e8e6, the `mqtt_socket_timer` function was modified so that instead of checking the presense of allocated `mud->pesp_conn` structure, `mud->connected` field was used on determining if the timer need to be disarmed. However, this is not entirely correct. If the TCP socket is actively connecting and haven't timed out yet, then `mud->connected` is also `false` and the timer will think the connection is broken and disarms itself. This has two consequences: * The connection timeout counter is no longer decremented and checked * After connection succeeds, keepalive heartbeat is no longer being sent (#3166). This is particularly noticeable in MQTT over TLS connections, because those usually takes longer than 1 second to finish and the timer would had chance to execute before connection is established This commit checks the presense of `pesp_conn->proto.tcp` pointer instead, which was allocated in the same place as the (old) `pesp_conn` struct, and according to my test indeed fixes the above issue. --- app/modules/mqtt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/modules/mqtt.c b/app/modules/mqtt.c index 584f6c8c..17a186b7 100644 --- a/app/modules/mqtt.c +++ b/app/modules/mqtt.c @@ -753,7 +753,7 @@ void mqtt_socket_timer(void *arg) if(mud == NULL) return; - if(mud->connected == 0){ + if(mud->pesp_conn.proto.tcp == NULL){ NODE_DBG("MQTT not connected\n"); os_timer_disarm(&mud->mqttTimer); return; @@ -1289,7 +1289,7 @@ static int mqtt_socket_close( lua_State* L ) espconn_status |= espconn_disconnect(&mud->pesp_conn); } } - mud->connected = 0; + mud->connected = false; while (mud->mqtt_state.pending_msg_q) { msg_destroy(msg_dequeue(&(mud->mqtt_state.pending_msg_q)));