From 30230bede737dda19dc5ed2b15bb37b70aeb71e1 Mon Sep 17 00:00:00 2001 From: Dengke Tang Date: Thu, 21 Jul 2022 16:12:53 -0700 Subject: [PATCH] fix the dead lock for stream manager (#384) --- include/aws/http/connection_manager.h | 2 ++ source/http2_stream_manager.c | 22 ++++++++++++++-------- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/include/aws/http/connection_manager.h b/include/aws/http/connection_manager.h index eec96f7be..4153e72c3 100644 --- a/include/aws/http/connection_manager.h +++ b/include/aws/http/connection_manager.h @@ -171,6 +171,8 @@ void aws_http_connection_manager_acquire_connection( /* * Returns a connection back to the manager. All acquired connections must * eventually be released back to the manager in order to avoid a resource leak. + * + * Note: it can lead to another acquired callback to be invoked within the thread. */ AWS_HTTP_API int aws_http_connection_manager_release_connection( diff --git a/source/http2_stream_manager.c b/source/http2_stream_manager.c index 87a54214e..9ce43070c 100644 --- a/source/http2_stream_manager.c +++ b/source/http2_stream_manager.c @@ -164,7 +164,7 @@ static void s_sm_try_assign_connection_to_pending_stream_acquisition_synced( chosen_connection->num_streams_assigned++; STREAM_MANAGER_LOGF( - TRACE, + DEBUG, stream_manager, "Picking connection:%p for acquisition:%p. Streams assigned to the connection=%" PRIu32 "", (void *)chosen_connection->connection, @@ -216,7 +216,7 @@ static void s_sm_try_assign_connection_to_pending_stream_acquisition_synced( chosen_connection->num_streams_assigned++; STREAM_MANAGER_LOGF( - TRACE, + DEBUG, stream_manager, "Picking connection:%p for acquisition:%p. Streams assigned to the connection=%" PRIu32 "", (void *)chosen_connection->connection, @@ -321,7 +321,7 @@ static void s_check_new_connections_needed_synced(struct aws_http2_stream_manage /* Update the number of connections we acquiring */ s_sm_count_increase_synced(stream_manager, AWS_SMCT_CONNECTIONS_ACQUIRING, work->new_connections); STREAM_MANAGER_LOGF( - TRACE, + DEBUG, stream_manager, "number of acquisition that waiting for connections to use=%zu. connection acquiring=%zu, connection held=%zu, " "max connection=%zu", @@ -354,7 +354,7 @@ static void s_aws_http2_stream_manager_build_transaction_synced(struct aws_http2 /* Cannot find any connection, push it back to the front and break the loop */ aws_linked_list_push_front(&stream_manager->synced_data.pending_stream_acquisitions, node); STREAM_MANAGER_LOGF( - TRACE, + DEBUG, stream_manager, "acquisition:%p cannot find any connection to use.", (void *)pending_stream_acquisition); @@ -570,6 +570,7 @@ static void s_sm_on_connection_acquired(struct aws_http_connection *connection, STREAM_MANAGER_LOGF(TRACE, stream_manager, "connection=%p acquired from connection manager", (void *)connection); int re_error = 0; int stream_fail_error_code = AWS_ERROR_SUCCESS; + bool should_release_connection = false; struct aws_linked_list stream_acquisitions_to_fail; aws_linked_list_init(&stream_acquisitions_to_fail); s_aws_stream_management_transaction_init(&work, stream_manager); @@ -591,7 +592,7 @@ static void s_sm_on_connection_acquired(struct aws_http_connection *connection, stream_manager, "Unexpected HTTP version acquired, release the connection=%p acquired immediately", (void *)connection); - re_error |= aws_http_connection_manager_release_connection(stream_manager->connection_manager, connection); + should_release_connection = true; s_sm_on_connection_acquired_failed_synced(stream_manager, &stream_acquisitions_to_fail); stream_fail_error_code = AWS_ERROR_HTTP_STREAM_MANAGER_UNEXPECTED_HTTP_VERSION; } else if (stream_manager->synced_data.state != AWS_H2SMST_READY) { @@ -601,7 +602,7 @@ static void s_sm_on_connection_acquired(struct aws_http_connection *connection, "shutting down, release the connection=%p acquired immediately", (void *)connection); /* Release the acquired connection */ - re_error |= aws_http_connection_manager_release_connection(stream_manager->connection_manager, connection); + should_release_connection = true; } else if (stream_manager->synced_data.internal_refcount_stats[AWS_SMCT_PENDING_ACQUISITION] == 0) { STREAM_MANAGER_LOGF( DEBUG, @@ -609,7 +610,7 @@ static void s_sm_on_connection_acquired(struct aws_http_connection *connection, "No pending acquisition, release the connection=%p acquired immediately", (void *)connection); /* Release the acquired connection */ - re_error |= aws_http_connection_manager_release_connection(stream_manager->connection_manager, connection); + should_release_connection = true; } else { struct aws_h2_sm_connection *sm_connection = s_sm_connection_new(stream_manager, connection); bool added = false; @@ -622,6 +623,11 @@ static void s_sm_on_connection_acquired(struct aws_http_connection *connection, s_unlock_synced_data(stream_manager); } /* END CRITICAL SECTION */ + if (should_release_connection) { + STREAM_MANAGER_LOGF(DEBUG, stream_manager, "Releasing connection: %p", (void *)connection); + re_error |= aws_http_connection_manager_release_connection(stream_manager->connection_manager, connection); + } + AWS_ASSERT(!re_error && "connection acquired callback fails with programming errors"); (void)re_error; @@ -957,7 +963,7 @@ static void s_aws_http2_stream_manager_execute_transaction(struct aws_http2_stre /* Step 3: Acquire connections if needed */ if (work->new_connections) { - STREAM_MANAGER_LOGF(TRACE, stream_manager, "acquiring %zu new connections", work->new_connections); + STREAM_MANAGER_LOGF(DEBUG, stream_manager, "acquiring %zu new connections", work->new_connections); } for (size_t i = 0; i < work->new_connections; ++i) { aws_http_connection_manager_acquire_connection(