Fix memory leak during signing (#596)

**Issue:** (internal ticket V1637484435)

When doing SigV4A signing from aws-sdk-js-v3, memory usage gradually increases due to memory leaks.

**Description of changes:**
Fix leak that occurred if headers were queried repeatedly.

**Future Work:**
All native unit tests should check that they aren't leaking memory. I spent a few hours trying to add such tests, but couldn't get it working in a reliable way. I didn't want this to hold back the fix, so leaving it for another day. See these blog posts for hints on various techniques NodeJS has used for leak testing through the years: https://joyeecheung.github.io/blog/2024/03/17/memory-leak-testing-v8-node-js-1/, https://joyeecheung.github.io/blog/2024/03/17/memory-leak-testing-v8-node-js-2/

Author: graebm

source/http_connection.c

@@ -149,9 +149,12 @@ struct http_connection_binding {
 /* finalizer called when node cleans up this object */
 static void s_http_connection_from_manager_binding_finalize(napi_env env, void *finalize_data, void *finalize_hint) {
     (void)finalize_hint;
-    (void)env;
     struct http_connection_binding *binding = finalize_data;
 
+    if (binding->node_external != NULL) {
+        napi_delete_reference(env, binding->node_external);
+    }
+
     /* no release call, the http_client_connection_manager has already released it */
     aws_mem_release(binding->allocator, binding);
 }

source/http_connection_manager.c

@@ -27,8 +27,10 @@ struct aws_http_connection_manager *aws_napi_get_http_connection_manager(
 
 static void s_http_connection_manager_finalize(napi_env env, void *finalize_data, void *finalize_hint) {
     (void)finalize_hint;
-    (void)env;
     struct http_connection_manager_binding *binding = finalize_data;
+    if (binding->node_external != NULL) {
+        napi_delete_reference(env, binding->node_external);
+    }
     aws_mem_release(binding->allocator, binding);
 }
 

source/http_message.c

@@ -79,29 +79,29 @@ napi_status aws_napi_http_message_bind(napi_env env, napi_value exports) {
 
 struct http_request_binding {
     struct aws_http_message *native;
-    struct aws_allocator *allocator;
 
     napi_ref node_headers;
 };
 
-/* Need a special finalizer to avoid releasing a request object we don't own */
-static void s_napi_wrapped_http_request_finalize(napi_env env, void *finalize_data, void *finalize_hint) {
-    (void)env;
+static void s_napi_http_request_finalize(napi_env env, void *finalize_data, void *finalize_hint) {
     (void)finalize_hint;
-
     struct http_request_binding *binding = finalize_data;
-    struct aws_allocator *allocator = binding->allocator;
+    struct aws_allocator *allocator = aws_napi_get_allocator();
+
+    if (binding->node_headers != NULL) {
+        napi_delete_reference(env, binding->node_headers);
+    }
 
+    aws_http_message_release(binding->native);
     aws_mem_release(allocator, binding);
 }
 
 napi_status aws_napi_http_message_wrap(napi_env env, struct aws_http_message *message, napi_value *result) {
 
     struct http_request_binding *binding =
         aws_mem_calloc(aws_napi_get_allocator(), 1, sizeof(struct http_request_binding));
-    binding->native = message;
-    binding->allocator = aws_napi_get_allocator();
-    return aws_napi_wrap(env, &s_request_class_info, binding, s_napi_wrapped_http_request_finalize, result);
+    binding->native = aws_http_message_acquire(message);
+    return aws_napi_wrap(env, &s_request_class_info, binding, s_napi_http_request_finalize, result);
 }
 
 struct aws_http_message *aws_napi_http_message_unwrap(napi_env env, napi_value js_object) {
@@ -115,20 +115,6 @@ struct aws_http_message *aws_napi_http_message_unwrap(napi_env env, napi_value j
  * Constructor
  **********************************************************************************************************************/
 
-static void s_napi_http_request_finalize(napi_env env, void *finalize_data, void *finalize_hint) {
-    (void)env;
-
-    struct http_request_binding *binding = finalize_data;
-    struct aws_allocator *allocator = finalize_hint;
-
-    if (binding->node_headers != NULL) {
-        napi_delete_reference(env, binding->node_headers);
-    }
-
-    aws_http_message_destroy(binding->native);
-    aws_mem_release(allocator, binding);
-}
-
 static napi_value s_request_constructor(napi_env env, const struct aws_napi_callback_info *cb_info) {
 
     struct aws_allocator *alloc = aws_napi_get_allocator();
@@ -167,7 +153,7 @@ static napi_value s_request_constructor(napi_env env, const struct aws_napi_call
     }
 
     napi_value node_this = cb_info->native_this;
-    AWS_NAPI_CALL(env, napi_wrap(env, node_this, binding, s_napi_http_request_finalize, alloc, NULL), {
+    AWS_NAPI_CALL(env, napi_wrap(env, node_this, binding, s_napi_http_request_finalize, NULL, NULL), {
         napi_throw_error(env, NULL, "Failed to wrap HttpRequest");
         goto cleanup;
     });
@@ -177,7 +163,7 @@ static napi_value s_request_constructor(napi_env env, const struct aws_napi_call
 cleanup:
     if (binding) {
         if (binding->native) {
-            aws_http_message_destroy(binding->native);
+            aws_http_message_release(binding->native);
         }
         aws_mem_release(alloc, binding);
     }
@@ -238,15 +224,16 @@ static napi_value s_request_headers_get(napi_env env, void *native_this) {
 
     napi_value result = NULL;
     if (binding->node_headers) {
+        /* This HTTP request already has a reference to the Node object for the headers */
         AWS_NAPI_ENSURE(env, napi_get_reference_value(env, binding->node_headers, &result));
     } else {
+        /* There is no Node object for these headers.
+         * Create a Node object, and a reference to it, and store that reference on this HTTP request */
         struct aws_http_headers *headers = aws_http_message_get_headers(binding->native);
         AWS_NAPI_ENSURE(env, aws_napi_http_headers_wrap(env, headers, &result));
+        AWS_NAPI_ENSURE(env, napi_create_reference(env, result, 1, &binding->node_headers));
     }
 
-    /* Store the value for later */
-    AWS_NAPI_ENSURE(env, napi_create_reference(env, result, 1, &binding->node_headers));
-
     return result;
 }
 

source/http_stream.c

@@ -74,7 +74,7 @@ static void s_on_response_call(napi_env env, napi_value on_response, void *conte
     }
 
     /* clean up the response buffer */
-    aws_http_message_destroy(binding->response);
+    aws_http_message_release(binding->response);
     binding->response = NULL;
 }