[fix] h2 double complete (#558)

Author: TingDaoK

include/aws/http/private/h2_stream.h

@@ -118,6 +118,9 @@ struct aws_h2_stream {
          * asleep. When stream needs to be awaken, moving the stream back to the outgoing_streams_list and set this bool
          * to false */
         bool waiting_for_writes;
+        /* Set when s_stream_complete has been called. Prevents double-completion
+         * (e.g. GOAWAY completes stream, then pending cancel cross-thread task fires) */
+        bool is_complete;
     } thread_data;
 
     /* Any thread may touch this data, but the lock must be held (unless it's an atomic) */

source/h2_connection.c

@@ -1868,6 +1868,13 @@ static void s_handler_installed(struct aws_channel_handler *handler, struct aws_
 static void s_stream_complete(struct aws_h2_connection *connection, struct aws_h2_stream *stream, int error_code) {
     AWS_PRECONDITION(aws_channel_thread_is_callers_thread(connection->base.channel_slot->channel));
 
+    /* Guard against double-completion (e.g. GOAWAY completes stream, then pending cancel task fires) */
+    if (stream->thread_data.is_complete) {
+        AWS_H2_STREAM_LOG(DEBUG, stream, "stream already completed, ignoring.");
+        return;
+    }
+    stream->thread_data.is_complete = true;
+
     /* Nice logging */
     if (error_code) {
         AWS_H2_STREAM_LOGF(

tests/CMakeLists.txt

@@ -712,6 +712,7 @@ add_net_test_case(h2_sm_mock_complete_stream)
 add_net_test_case(h2_sm_mock_ideal_num_streams)
 add_net_test_case(h2_sm_mock_large_ideal_num_streams)
 add_net_test_case(h2_sm_mock_goaway)
+add_net_test_case(h2_sm_mock_cancel_after_goaway_no_double_complete)
 add_net_test_case(h2_sm_connection_ping)
 add_net_test_case(h2_sm_with_flow_control_err)
 add_net_test_case(h2_sm_with_initial_settings)

tests/test_stream_manager.c

@@ -1152,6 +1152,63 @@ TEST_CASE(h2_sm_mock_goaway) {
     return s_tester_clean_up();
 }
 
+/* Test that cancelling a stream after GOAWAY does not crash from double stream completion.
+ * Reproduces P428540711: cancel() schedules cross-thread task, but s_finish_shutdown already
+ * completed the stream. The cross-thread task then invokes on_complete a second time. */
+TEST_CASE(h2_sm_mock_cancel_after_goaway_no_double_complete) {
+    (void)ctx;
+    struct sm_tester_options options = {
+        .max_connections = 1,
+        .max_concurrent_streams_per_connection = 1,
+        .alloc = allocator,
+    };
+    ASSERT_SUCCESS(s_tester_init(&options));
+    s_override_cm_connect_function(s_aws_http_connection_manager_create_connection_sync_mock);
+
+    /* Acquire 1 stream */
+    ASSERT_SUCCESS(s_sm_stream_acquiring(1));
+    ASSERT_SUCCESS(s_wait_on_fake_connection_count(1));
+    s_drain_all_fake_connection_testing_channel();
+    ASSERT_SUCCESS(s_wait_on_streams_acquired_count(1));
+    ASSERT_INT_EQUALS(0, s_tester.acquiring_stream_errors);
+
+    struct sm_fake_connection *fake_connection = s_get_fake_connection(0);
+    ASSERT_SUCCESS(h2_fake_peer_send_connection_preface_default_settings(&fake_connection->peer));
+    testing_channel_drain_queued_tasks(&fake_connection->testing_channel);
+
+    /* Get the stream handle */
+    struct aws_http_stream *stream = NULL;
+    aws_array_list_front(&s_tester.streams, &stream);
+    ASSERT_NOT_NULL(stream);
+
+    /* Send GOAWAY with last_stream_id=0, meaning our stream (id=1) is rejected */
+    struct aws_byte_cursor debug_info;
+    AWS_ZERO_STRUCT(debug_info);
+    struct aws_h2_frame *goaway_frame =
+        aws_h2_frame_new_goaway(allocator, 0 /*last_stream_id*/, AWS_HTTP2_ERR_INTERNAL_ERROR, debug_info);
+    ASSERT_SUCCESS(h2_fake_peer_send_frame(&fake_connection->peer, goaway_frame));
+
+    /* Cancel the stream BEFORE draining tasks.
+     * This schedules the cross-thread work task (reset_called=true).
+     * When we drain, both the GOAWAY processing (which completes the stream)
+     * and the cancel cross-thread task will run, causing double on_complete. */
+    aws_http_stream_cancel_default_error(stream);
+
+    /* User releases their ref on the stream (simulates real usage: cancel then release).
+     * The stream must stay alive until the cross-thread task completes. */
+    aws_http_stream_release(stream);
+    aws_array_list_clear(&s_tester.streams);
+
+    /* Drain tasks - this should NOT crash from double stream completion or use-after-free */
+    testing_channel_drain_queued_tasks(&fake_connection->testing_channel);
+
+    /* Stream should be completed with error at least once */
+    ASSERT_TRUE(s_tester.stream_completed_count >= 1);
+    ASSERT_TRUE(s_tester.stream_complete_errors >= 1);
+
+    return s_tester_clean_up();
+}
+
 /* Test that PING works as expected. */
 TEST_CASE(h2_sm_connection_ping) {
     (void)ctx;