Fix naming for on_termination functions and types

Move on_termination call to a proper function.
Add test for on_termination.

include/aws/mqtt/client.h

@@ -164,7 +164,7 @@ typedef void(aws_mqtt_client_on_disconnect_fn)(struct aws_mqtt_client_connection
 /**
  * Signature of callback invoked on a connection destruction.
  */
-typedef void(aws_mqtt_client_on_termination_fn)(void *userdata);
+typedef void(aws_mqtt_client_on_connection_termination_fn)(void *userdata);
 
 /**
  * Function to invoke when the websocket handshake request transformation completes.
@@ -519,9 +519,9 @@ int aws_mqtt_client_connection_set_on_any_publish_handler(
  * \param[in] on_termination_ud Userdata for on_termination.
  */
 AWS_MQTT_API
-int aws_mqtt_client_connection_set_termination_handler(
+int aws_mqtt_client_connection_set_connection_termination_handler(
     struct aws_mqtt_client_connection *connection,
-    aws_mqtt_client_on_termination_fn *on_termination,
+    aws_mqtt_client_on_connection_termination_fn *on_termination,
     void *on_termination_ud);
 
 /**

include/aws/mqtt/private/client_impl.h

@@ -250,7 +250,7 @@ struct aws_mqtt_client_connection_311_impl {
     void *on_any_publish_ud;
     aws_mqtt_client_on_disconnect_fn *on_disconnect;
     void *on_disconnect_ud;
-    aws_mqtt_client_on_termination_fn *on_termination;
+    aws_mqtt_client_on_connection_termination_fn *on_termination;
     void *on_termination_ud;
     aws_mqtt_on_operation_statistics_fn *on_any_operation_statistics;
     void *on_any_operation_statistics_ud;

include/aws/mqtt/private/client_impl_shared.h

@@ -62,9 +62,9 @@ struct aws_mqtt_client_connection_vtable {
         aws_mqtt_client_publish_received_fn *on_any_publish,
         void *on_any_publish_ud);
 
-    int (*set_termination_handler_fn)(
+    int (*set_connection_termination_handler_fn)(
         void *impl,
-        aws_mqtt_client_on_termination_fn *on_termination,
+        aws_mqtt_client_on_connection_termination_fn *on_termination,
         void *on_termination_ud);
 
     int (*connect_fn)(void *impl, const struct aws_mqtt_connection_options *connection_options);

source/client.c

@@ -794,6 +794,13 @@ static void s_mqtt_client_connection_destroy_final(struct aws_mqtt_client_connec
 
     AWS_LOGF_DEBUG(AWS_LS_MQTT_CLIENT, "id=%p: Destroying connection", (void *)connection);
 
+    aws_mqtt_client_on_connection_termination_fn *termination_handler = NULL;
+    void *termination_handler_user_data = NULL;
+    if (connection->on_termination != NULL) {
+        termination_handler = connection->on_termination;
+        termination_handler_user_data = connection->on_termination_ud;
+    }
+
     /* If the reconnect_task isn't freed, free it */
     if (connection->reconnect_task) {
         aws_mem_release(connection->reconnect_task->allocator, connection->reconnect_task);
@@ -848,6 +855,10 @@ static void s_mqtt_client_connection_destroy_final(struct aws_mqtt_client_connec
 
     /* Frees all allocated memory */
     aws_mem_release(connection->allocator, connection);
+
+    if (termination_handler != NULL) {
+        (*termination_handler)(termination_handler_user_data);
+    }
 }
 
 static void s_on_final_disconnect(struct aws_mqtt_client_connection *connection, void *userdata) {
@@ -864,14 +875,6 @@ static void s_mqtt_client_connection_start_destroy(struct aws_mqtt_client_connec
         "id=%p: Last refcount on connection has been released, start destroying the connection.",
         (void *)connection);
 
-    aws_mqtt_client_on_termination_fn *client_termination_handler = NULL;
-    void *client_termination_handler_user_data = NULL;
-    /* TODO Should it be in a critical section? */
-    if (connection->on_termination != NULL) {
-        client_termination_handler = connection->on_termination;
-        client_termination_handler_user_data = connection->on_termination_ud;
-    }
-
     { /* BEGIN CRITICAL SECTION */
         mqtt_connection_lock_synced_data(connection);
         if (connection->synced_data.state != AWS_MQTT_CLIENT_STATE_DISCONNECTED) {
@@ -899,10 +902,6 @@ static void s_mqtt_client_connection_start_destroy(struct aws_mqtt_client_connec
     if (call_destroy_final) {
         s_mqtt_client_connection_destroy_final(&connection->base);
     }
-
-    if (client_termination_handler != NULL) {
-        (*client_termination_handler)(client_termination_handler_user_data);
-    }
 }
 
 /*******************************************************************************
@@ -1172,9 +1171,9 @@ static int s_aws_mqtt_client_connection_311_set_on_any_publish_handler(
     return AWS_OP_SUCCESS;
 }
 
-static int s_aws_mqtt_client_connection_311_set_termination_handler(
+static int s_aws_mqtt_client_connection_311_set_connection_termination_handler(
     void *impl,
-    aws_mqtt_client_on_termination_fn *on_termination,
+    aws_mqtt_client_on_connection_termination_fn *on_termination,
     void *on_termination_ud) {
 
     struct aws_mqtt_client_connection_311_impl *connection = impl;
@@ -3195,7 +3194,7 @@ static struct aws_mqtt_client_connection_vtable s_aws_mqtt_client_connection_311
     .set_connection_interruption_handlers_fn = s_aws_mqtt_client_connection_311_set_connection_interruption_handlers,
     .set_connection_closed_handler_fn = s_aws_mqtt_client_connection_311_set_connection_closed_handler,
     .set_on_any_publish_handler_fn = s_aws_mqtt_client_connection_311_set_on_any_publish_handler,
-    .set_termination_handler_fn = s_aws_mqtt_client_connection_311_set_termination_handler,
+    .set_connection_termination_handler_fn = s_aws_mqtt_client_connection_311_set_connection_termination_handler,
     .connect_fn = s_aws_mqtt_client_connection_311_connect,
     .reconnect_fn = s_aws_mqtt_client_connection_311_reconnect,
     .disconnect_fn = s_aws_mqtt_client_connection_311_disconnect,

source/client_impl_shared.c

@@ -113,12 +113,12 @@ int aws_mqtt_client_connection_set_on_any_publish_handler(
     return (*connection->vtable->set_on_any_publish_handler_fn)(connection->impl, on_any_publish, on_any_publish_ud);
 }
 
-int aws_mqtt_client_connection_set_termination_handler(
+int aws_mqtt_client_connection_set_connection_termination_handler(
     struct aws_mqtt_client_connection *connection,
-    aws_mqtt_client_on_termination_fn *on_termination,
+    aws_mqtt_client_on_connection_termination_fn *on_termination,
     void *on_termination_ud) {
 
-    return (*connection->vtable->set_termination_handler_fn)(connection->impl, on_termination, on_termination_ud);
+    return (*connection->vtable->set_connection_termination_handler_fn)(connection->impl, on_termination, on_termination_ud);
 }
 
 int aws_mqtt_client_connection_connect(

tests/CMakeLists.txt

@@ -81,6 +81,7 @@ add_test_case(mqtt_connection_ping_double_scenario)
 add_test_case(mqtt_connection_close_callback_simple)
 add_test_case(mqtt_connection_close_callback_interrupted)
 add_test_case(mqtt_connection_close_callback_multi)
+add_test_case(mqtt_connection_termination_callback_simple)
 add_test_case(mqtt_connection_reconnection_backoff_stable)
 add_test_case(mqtt_connection_reconnection_backoff_unstable)
 add_test_case(mqtt_connection_reconnection_backoff_reset)

tests/v3/connection_state_test.c

@@ -80,6 +80,9 @@ struct mqtt_connection_state_test {
     size_t ops_completed;
     size_t expected_ops_completed;
     size_t connection_close_calls; /* All of the times on_connection_closed has been called */
+
+    bool termination_completed;
+    size_t connection_termination_calls;
 };
 
 static struct mqtt_connection_state_test test_data = {0};
@@ -3061,6 +3064,55 @@ AWS_TEST_CASE_FIXTURE(
     s_clean_up_mqtt_server_fn,
     &test_data)
 
+
+static bool s_is_termination_completed(void *arg) {
+    struct mqtt_connection_state_test *state_test_data = arg;
+    return state_test_data->termination_completed;
+}
+
+static void s_wait_for_termination_to_complete(struct mqtt_connection_state_test *state_test_data) {
+    aws_mutex_lock(&state_test_data->lock);
+    aws_condition_variable_wait_pred(
+        &state_test_data->cvar, &state_test_data->lock, s_is_termination_completed, state_test_data);
+    state_test_data->termination_completed = false;
+    aws_mutex_unlock(&state_test_data->lock);
+}
+
+static void s_on_connection_termination_fn(void *userdata) {
+    struct mqtt_connection_state_test *state_test_data = (struct mqtt_connection_state_test *)userdata;
+
+    aws_mutex_lock(&state_test_data->lock);
+    state_test_data->connection_termination_calls += 1;
+    aws_mutex_unlock(&state_test_data->lock);
+
+    state_test_data->termination_completed = true;
+}
+
+/**
+ * Test that the connection termination callback is fired for the connection that was not actually connected ever.
+ */
+static int s_test_mqtt_connection_termination_callback_simple_fn(struct aws_allocator *allocator, void *ctx) {
+    (void)allocator;
+    struct mqtt_connection_state_test *state_test_data = ctx;
+
+    aws_mqtt_client_connection_set_connection_termination_handler(
+        state_test_data->mqtt_connection, s_on_connection_termination_fn, state_test_data);
+
+    aws_mqtt_client_connection_release(state_test_data->mqtt_connection);
+
+    s_wait_for_termination_to_complete(state_test_data);
+    ASSERT_UINT_EQUALS(1, state_test_data->connection_termination_calls);
+
+    return AWS_OP_SUCCESS;
+}
+
+AWS_TEST_CASE_FIXTURE(
+    mqtt_connection_termination_callback_simple,
+    s_setup_mqtt_server_fn,
+    s_test_mqtt_connection_termination_callback_simple_fn,
+    s_clean_up_mqtt_server_fn,
+    &test_data)
+
 static int s_test_mqtt_connection_reconnection_backoff_stable(struct aws_allocator *allocator, void *ctx) {
 
     (void)allocator;