Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update callback order to aovid mqtt3 client callback race condition #316

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions source/client.c
Original file line number Diff line number Diff line change
Expand Up @@ -440,16 +440,16 @@ static void s_mqtt_client_shutdown(
AWS_LS_MQTT_CLIENT,
"id=%p: Caller requested disconnect from on_interrupted callback, aborting reconnect",
(void *)connection);
MQTT_CLIENT_CALL_CALLBACK(connection, on_disconnect);
MQTT_CLIENT_CALL_CALLBACK_ARGS(connection, on_closed, NULL);
MQTT_CLIENT_CALL_CALLBACK(connection, on_disconnect);
break;
case AWS_MQTT_CLIENT_STATE_DISCONNECTING:
AWS_LOGF_DEBUG(
AWS_LS_MQTT_CLIENT,
"id=%p: Disconnect completed, clearing request queue and calling callback",
(void *)connection);
MQTT_CLIENT_CALL_CALLBACK(connection, on_disconnect);
MQTT_CLIENT_CALL_CALLBACK_ARGS(connection, on_closed, NULL);
MQTT_CLIENT_CALL_CALLBACK(connection, on_disconnect);
break;
case AWS_MQTT_CLIENT_STATE_CONNECTING:
AWS_LOGF_TRACE(
Expand Down Expand Up @@ -727,8 +727,8 @@ static void s_attempt_reconnect(struct aws_task *task, void *userdata, enum aws_
/* Unlock the synced data, then potentially call the disconnect callback and release the connection */
mqtt_connection_unlock_synced_data(connection);
if (perform_full_destroy) {
MQTT_CLIENT_CALL_CALLBACK(connection, on_disconnect);
MQTT_CLIENT_CALL_CALLBACK_ARGS(connection, on_closed, NULL);
MQTT_CLIENT_CALL_CALLBACK(connection, on_disconnect);
aws_mqtt_client_connection_release(&connection->base);
}
return;
Expand Down
15 changes: 7 additions & 8 deletions source/v5/mqtt5_to_mqtt3_adapter.c
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,10 @@ static int s_aws_mqtt5_to_mqtt3_adapter_safe_lifecycle_handler(
(void *)adapter,
(int)adapter->adapter_state);

if (adapter->on_closed) {
(*adapter->on_closed)(&adapter->base, NULL, adapter->on_closed_user_data);
}

/* If an MQTT311-view user is waiting on a disconnect callback, invoke it */
if (adapter->on_disconnect) {
(*adapter->on_disconnect)(&adapter->base, adapter->on_disconnect_user_data);
Expand All @@ -560,10 +564,6 @@ static int s_aws_mqtt5_to_mqtt3_adapter_safe_lifecycle_handler(
adapter->on_disconnect_user_data = NULL;
}

if (adapter->on_closed) {
(*adapter->on_closed)(&adapter->base, NULL, adapter->on_closed_user_data);
}

/*
* Judgement call: If the mqtt5 client is stopped behind our back, it seems better to transition to the
* disconnected state (which only requires a connect() to restart) then stay in the STAY_CONNECTED state
Expand Down Expand Up @@ -650,13 +650,12 @@ static int s_aws_mqtt5_to_mqtt3_adapter_safe_disconnect_handler(
}

if (invoke_callbacks) {
if (disconnect_task->on_disconnect != NULL) {
(*disconnect_task->on_disconnect)(&adapter->base, disconnect_task->on_disconnect_user_data);
}

if (adapter->on_closed) {
(*adapter->on_closed)(&adapter->base, NULL, adapter->on_closed_user_data);
}
if (disconnect_task->on_disconnect != NULL) {
(*disconnect_task->on_disconnect)(&adapter->base, disconnect_task->on_disconnect_user_data);
}
}

return AWS_OP_SUCCESS;
Expand Down
12 changes: 6 additions & 6 deletions tests/v5/mqtt5_to_mqtt3_adapter_tests.c
Original file line number Diff line number Diff line change
Expand Up @@ -1002,8 +1002,8 @@ static int s_do_mqtt5to3_adapter_connect_success_disconnect_success_cycle(
aws_mqtt_client_connection_disconnect(
adapter, s_aws_mqtt5_to_mqtt3_adapter_test_fixture_record_disconnection_complete, &fixture);

s_wait_for_n_adapter_lifecycle_events(&fixture, AWS_MQTT3_LET_DISCONNECTION_COMPLETE, i + 1);
s_wait_for_n_adapter_lifecycle_events(&fixture, AWS_MQTT3_LET_CLOSED, i + 1);
s_wait_for_n_adapter_lifecycle_events(&fixture, AWS_MQTT3_LET_DISCONNECTION_COMPLETE, i + 1);

struct aws_mqtt3_lifecycle_event expected_event_sequence[] = {
{
Expand All @@ -1013,10 +1013,10 @@ static int s_do_mqtt5to3_adapter_connect_success_disconnect_success_cycle(
.type = AWS_MQTT3_LET_CONNECTION_COMPLETE,
},
{
.type = AWS_MQTT3_LET_DISCONNECTION_COMPLETE,
.type = AWS_MQTT3_LET_CLOSED,
},
{
.type = AWS_MQTT3_LET_CLOSED,
.type = AWS_MQTT3_LET_DISCONNECTION_COMPLETE,
},
};
size_t sequence_size = AWS_ARRAY_SIZE(expected_event_sequence);
Expand Down Expand Up @@ -1220,10 +1220,10 @@ static int s_verify_bad_connectivity_callbacks(struct aws_mqtt5_to_mqtt3_adapter

struct aws_mqtt3_lifecycle_event expected_events_end[] = {
{
.type = AWS_MQTT3_LET_DISCONNECTION_COMPLETE,
.type = AWS_MQTT3_LET_CLOSED,
},
{
.type = AWS_MQTT3_LET_CLOSED,
.type = AWS_MQTT3_LET_DISCONNECTION_COMPLETE
},
};

Expand Down Expand Up @@ -1899,8 +1899,8 @@ static int s_mqtt5to3_adapter_connect_success_disconnect_success_disconnect_succ
aws_mqtt_client_connection_disconnect(
adapter, s_aws_mqtt5_to_mqtt3_adapter_test_fixture_record_disconnection_complete, &fixture);

s_wait_for_n_adapter_lifecycle_events(&fixture, AWS_MQTT3_LET_DISCONNECTION_COMPLETE, 3);
s_wait_for_n_adapter_lifecycle_events(&fixture, AWS_MQTT3_LET_CLOSED, 1);
s_wait_for_n_adapter_lifecycle_events(&fixture, AWS_MQTT3_LET_DISCONNECTION_COMPLETE, 3);

aws_mqtt5_to_mqtt3_adapter_test_fixture_clean_up(&fixture);
aws_mqtt_library_clean_up();
Expand Down
Loading