Fix memory leaks caused from the http_connection

[anagno]

Hello everyone,

Thank you for your great work!
This PR is trying to resolve some memory leaks that exist in the Crow. It is related with:
• #546
• #552
• #539

From my investigation I could found 3 distinct memory leaks/memory mismanagement.

The first one is related with the invalid paths. For example, if we try in the basic example to curl using curl localhost:18080/hello/invalid, this request will cause a memory leak, because the http_connection will not be de-allocated.
This memory request is quite easy to resolve by using the patch below (if we do not want to use the full blown solution with the shared_from_this that is being used in this PR):

diff --git a/include/crow/http_connection.h b/include/crow/http_connection.h
index 7f8a2928..4e09d7e6 100644
--- a/include/crow/http_connection.h
+++ b/include/crow/http_connection.h
@@ -541,8 +541,8 @@ namespace crow
                           adaptor_.shutdown_write();
                           adaptor_.close();
                           CROW_LOG_DEBUG << this << " from write(1)";
-                          check_destroy();
                       }
+                      check_destroy();
                   }
                   else
                   {

The second memory mismanagement is located in https://github.com/CrowCpp/Crow/blob/master/include/crow/routing.h#L1721. The lambda is referencing a local variable, i.e. glob_completion_handler. Probably this handler should be moved in the new lambda and not be referenced.

diff --git a/include/crow/routing.h b/include/crow/routing.h
index fe05ce9c..c17f7a6d 100644
--- a/include/crow/routing.h
+++ b/include/crow/routing.h
@@ -1715,7 +1715,7 @@ namespace crow
                     return;
                 }
 
-                res.complete_request_handler_ = [&rule, &ctx, &container, &req, &res, &glob_completion_handler] {
+                res.complete_request_handler_ = [&rule, &ctx, &container, &req, &res, glob_completion_handler = std::move(glob_completion_handler)] {
                     detail::middleware_call_criteria_dynamic<true> crit_bwd(rule->mw_indices_.indices());
 
                     detail::after_handlers_call_helper<

The third memory leak is because of the raw pointer in http_server. That one is more complicated to resolve. I am no asio expert, but I think the only way to resolve it is by using shared_from_this (or start tracking all the connections as suggested also in #552). The problem is that we never release the first initial connection, that is created and then blocks waiting the other connections. The connections after the first initial one, are handled correct. But that initial connection will always be leaked. I could not find a way with asio to execute the initial request and then release the memory. Is this behavior by design? I am guessing this leak, will only be visible if multiple resets (from the same app) of the server happen. Otherwise it will not be a memory leak, that will be increasing with time.

My PR is introducing is shared_from_this to track the lifespam of the connections and then we also release the memory from the first initial connection. Since we are using shared_from_this, it also fixes the first 2 problems.

Is the analysis correct? Does the PR match the architecture of the project or should I just create a smaller PR, with the first 2 fixes?

Thank you in advance and kind regards,
Vasileios

closes #546
closes #552
closes #539

[The-EDev]

Sorry about the comment updates, I was trying to link the issues you mentioned.

[anagno]

@The-EDev Should I remove the build for the macos-10.15 ? I think it is not supported anymore, because of the actions/runner-images#5583

[anagno]

@The-EDev Any comments on this one? Is anything else missing ?

[The-EDev]

My apologies, I haven't had much time to look at Crow, I went down a rabbit-hole about github actions and how much they cost. I'm not sure if it's a good idea to add gh actions changes in this PR.

[anagno]

My apologies, I haven't had much time to look at Crow, I went down a rabbit-hole about github actions and how much they cost. I'm not sure if it's a good idea to add gh actions changes in this PR.

I removed the extra commit. If you think it is useful, I can open an another PR afterwards.

[The-EDev]

All done, Thank you very much for your work!

[anagno]

All done, Thank you very much for your work!

Thank you for this great project !

[anagno]

@The-EDev Does it make sense to also have a tag/release with this fix? If yes, do you want me to do something ?