Fixes for connection hangs and crashes (#60)

* A connection which is already disconnected in shutdown should try to reconnect, because that means it was hung up on

* channel shutdown complete event must ALWAYS be called from channel shutdown callbacks

* If the socket was connected, go into the reconnect cycle

* Re-worked reconnect detection logic

* Ensure that on_connection_complete is called for the first completed connection, even if it's a reconnect

* documented connection_count

include/aws/mqtt/private/client_impl.h

@@ -161,6 +161,11 @@ struct aws_mqtt_client_connection {
         bool retain;
         struct aws_byte_buf payload;
     } will;
+
+    /* number of times this connection has successfully CONNACK-ed, used
+     * to ensure on_connection_completed is sent on the first completed
+     * CONNECT/CONNACK cycle */
+    size_t connection_count;
 };
 
 struct aws_channel_handler_vtable *aws_mqtt_get_client_channel_vtable(void);

source/client.c

@@ -210,6 +210,12 @@ static void s_mqtt_client_shutdown(
 
     /* Always clear slot, as that's what's been shutdown */
     if (connection->slot) {
+        if (connection->state == AWS_MQTT_CLIENT_STATE_CONNECTING) {
+            /* if there was a slot and we were connecting, the socket at least
+             * managed to connect (and was likely hung up gracefully during setup)
+             * so this should behave like a dropped connection */
+            connection->state = AWS_MQTT_CLIENT_STATE_CONNECTED;
+        }
         aws_channel_slot_remove(connection->slot);
         connection->slot = NULL;
     }
@@ -233,7 +239,8 @@ static void s_mqtt_client_shutdown(
 
         assert(
             connection->state == AWS_MQTT_CLIENT_STATE_CONNECTED ||
-            connection->state == AWS_MQTT_CLIENT_STATE_RECONNECTING);
+            connection->state == AWS_MQTT_CLIENT_STATE_RECONNECTING ||
+            connection->state == AWS_MQTT_CLIENT_STATE_DISCONNECTED);
 
         if (connection->state == AWS_MQTT_CLIENT_STATE_CONNECTED) {
 
@@ -243,7 +250,8 @@ static void s_mqtt_client_shutdown(
 
         assert(
             connection->state == AWS_MQTT_CLIENT_STATE_RECONNECTING ||
-            connection->state == AWS_MQTT_CLIENT_STATE_DISCONNECTING);
+            connection->state == AWS_MQTT_CLIENT_STATE_DISCONNECTING ||
+            connection->state == AWS_MQTT_CLIENT_STATE_DISCONNECTED);
 
         /* This will only be true if the user called disconnect from the on_interrupted callback */
         if (connection->state == AWS_MQTT_CLIENT_STATE_DISCONNECTING) {
@@ -493,6 +501,7 @@ int aws_mqtt_client_connection_connect(
     connection->state = AWS_MQTT_CLIENT_STATE_CONNECTING;
     connection->clean_session = connection_options->clean_session;
     connection->keep_alive_time_secs = connection_options->keep_alive_time_secs;
+    connection->connection_count = 0;
 
     if (!connection_options->ping_timeout_ms) {
         connection->request_timeout_ns = s_default_request_timeout_ns;

source/client_channel_handler.c

@@ -87,8 +87,12 @@ static int s_packet_handler_connack(
     const bool was_reconnecting = connection->state == AWS_MQTT_CLIENT_STATE_RECONNECTING;
 
     connection->state = AWS_MQTT_CLIENT_STATE_CONNECTED;
+    connection->connection_count++;
 
-    if (was_reconnecting) {
+    /* It is possible for a connection to complete, and a hangup to occur before the
+     * CONNECT/CONNACK cycle completes. In that case, we must deliver on_connection_complete
+     * on the first successful CONNACK or user code will never think it's connected */
+    if (was_reconnecting && connection->connection_count > 1) {
         MQTT_CLIENT_CALL_CALLBACK_ARGS(connection, on_resumed, connack.connect_return_code, connack.session_present);
     } else {
         MQTT_CLIENT_CALL_CALLBACK_ARGS(
@@ -425,22 +429,20 @@ static int s_shutdown(
                 struct aws_mqtt_packet_connection disconnect;
                 aws_mqtt_packet_disconnect_init(&disconnect);
 
+                /* if any of these fail, we don't care, because we're disconnecting anyway */
                 struct aws_io_message *message = mqtt_get_message_for_packet(connection, &disconnect.fixed_header);
                 if (!message) {
-                    return AWS_OP_ERR;
+                    goto done;
                 }
-
                 if (aws_mqtt_packet_connection_encode(&message->message_data, &disconnect)) {
-                    return AWS_OP_ERR;
-                }
-
-                if (aws_channel_slot_send_message(slot, message, AWS_CHANNEL_DIR_WRITE)) {
-                    return AWS_OP_ERR;
+                    goto done;
                 }
+                aws_channel_slot_send_message(slot, message, AWS_CHANNEL_DIR_WRITE);
             }
         }
     }
 
+done:
     return aws_channel_slot_on_handler_shutdown_complete(slot, dir, error_code, free_scarce_resources_immediately);
 }