fix the dead lock for stream manager (#384)

awslabs · Jul 21, 2022 · 30230be · 30230be
1 parent c8fc870
commit 30230be
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 8 deletions.
diff --git 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
@@ -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,15 +602,15 @@ 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,
                 stream_manager,
                 "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(