Incremental memory management improvements.

Author: kinyoklion

libs/internal/src/network/curl_requester.cpp

@@ -93,7 +93,7 @@ void CurlRequester::PerformRequestWithMulti(std::shared_ptr<CurlMultiManager> mu
         return;
     }
 
-    CURL* curl = curl_easy_init();
+    std::shared_ptr<CURL>curl (curl_easy_init(), curl_easy_cleanup);
     if (!curl) {
         cb(HttpResult(kErrorCurlInit));
         return;
@@ -102,23 +102,14 @@ void CurlRequester::PerformRequestWithMulti(std::shared_ptr<CurlMultiManager> mu
     // Create context to hold data for this request
     // This will be cleaned up in the completion callback
     struct RequestContext {
-        CURL* curl;
         std::string url;
         std::string body; // Keep body alive
         std::string response_body;
         HttpResult::HeadersType response_headers;
         std::function<void(const HttpResult&)> callback;
-
-        ~RequestContext() {
-            // Headers are managed by CurlMultiManager
-            if (curl) {
-                curl_easy_cleanup(curl);
-            }
-        }
     };
 
     auto ctx = std::make_shared<RequestContext>();
-    ctx->curl = curl;
     ctx->callback = std::move(cb);
 
     // Headers will be managed by CurlMultiManager
@@ -144,28 +135,28 @@ void CurlRequester::PerformRequestWithMulti(std::shared_ptr<CurlMultiManager> mu
     ctx->url = request.Url();
 
     // Set URL
-    CURL_SETOPT_CHECK(curl, CURLOPT_URL, ctx->url.c_str());
+    CURL_SETOPT_CHECK(curl.get(), CURLOPT_URL, ctx->url.c_str());
 
     // Set HTTP method
     if (request.Method() == HttpMethod::kPost) {
         // Basically CURLOPT_POST is a flag that indicates this is a post.
         // Passing 1 enables this flag.
         // This will also set a content type, but the headers for the request
         // should override that with the correct value.
-        CURL_SETOPT_CHECK(curl, CURLOPT_POST, 1L);
+        CURL_SETOPT_CHECK(curl.get(), CURLOPT_POST, 1L);
     } else if (request.Method() == HttpMethod::kPut) {
-        CURL_SETOPT_CHECK(curl, CURLOPT_CUSTOMREQUEST, kHttpMethodPut);
+        CURL_SETOPT_CHECK(curl.get(), CURLOPT_CUSTOMREQUEST, kHttpMethodPut);
     } else if (request.Method() == HttpMethod::kReport) {
-        CURL_SETOPT_CHECK(curl, CURLOPT_CUSTOMREQUEST, kHttpMethodReport);
+        CURL_SETOPT_CHECK(curl.get(), CURLOPT_CUSTOMREQUEST, kHttpMethodReport);
     } else if (request.Method() == HttpMethod::kGet) {
-        CURL_SETOPT_CHECK(curl, CURLOPT_HTTPGET, 1L);
+        CURL_SETOPT_CHECK(curl.get(), CURLOPT_HTTPGET, 1L);
     }
 
     // Set request body if present
     if (request.Body().has_value()) {
         ctx->body = request.Body().value();
-        CURL_SETOPT_CHECK(curl, CURLOPT_POSTFIELDS, ctx->body.c_str());
-        CURL_SETOPT_CHECK(curl, CURLOPT_POSTFIELDSIZE, ctx->body.size());
+        CURL_SETOPT_CHECK(curl.get(), CURLOPT_POSTFIELDS, ctx->body.c_str());
+        CURL_SETOPT_CHECK(curl.get(), CURLOPT_POSTFIELDSIZE, ctx->body.size());
     }
 
     // Set headers
@@ -183,31 +174,31 @@ void CurlRequester::PerformRequestWithMulti(std::shared_ptr<CurlMultiManager> mu
         headers = appendResult;
     }
     if (headers) {
-        CURL_SETOPT_CHECK(curl, CURLOPT_HTTPHEADER, headers);
+        CURL_SETOPT_CHECK(curl.get(), CURLOPT_HTTPHEADER, headers);
     }
 
     // Set timeouts with millisecond precision
     const long connect_timeout_ms = request.Properties().ConnectTimeout().count();
     const long response_timeout_ms = request.Properties().ResponseTimeout().count();
 
-    CURL_SETOPT_CHECK(curl, CURLOPT_CONNECTTIMEOUT_MS, connect_timeout_ms > 0 ? connect_timeout_ms : 30000L);
-    CURL_SETOPT_CHECK(curl, CURLOPT_TIMEOUT_MS, response_timeout_ms > 0 ? response_timeout_ms : 60000L);
+    CURL_SETOPT_CHECK(curl.get(), CURLOPT_CONNECTTIMEOUT_MS, connect_timeout_ms > 0 ? connect_timeout_ms : 30000L);
+    CURL_SETOPT_CHECK(curl.get(), CURLOPT_TIMEOUT_MS, response_timeout_ms > 0 ? response_timeout_ms : 60000L);
 
     // Set TLS options
     using VerifyMode = config::shared::built::TlsOptions::VerifyMode;
     if (tls_options.PeerVerifyMode() == VerifyMode::kVerifyNone) {
-        CURL_SETOPT_CHECK(curl, CURLOPT_SSL_VERIFYPEER, 0L);
-        CURL_SETOPT_CHECK(curl, CURLOPT_SSL_VERIFYHOST, 0L);
+        CURL_SETOPT_CHECK(curl.get(), CURLOPT_SSL_VERIFYPEER, 0L);
+        CURL_SETOPT_CHECK(curl.get(), CURLOPT_SSL_VERIFYHOST, 0L);
     } else {
-        CURL_SETOPT_CHECK(curl, CURLOPT_SSL_VERIFYPEER, 1L);
+        CURL_SETOPT_CHECK(curl.get(), CURLOPT_SSL_VERIFYPEER, 1L);
         // 1 or 2 seem to basically be the same, but the documentation says to
         // use 2, and that it would default to 2.
         // https://curl.se/libcurl/c/CURLOPT_SSL_VERIFYHOST.html
-        CURL_SETOPT_CHECK(curl, CURLOPT_SSL_VERIFYHOST, 2L);
+        CURL_SETOPT_CHECK(curl.get(), CURLOPT_SSL_VERIFYHOST, 2L);
 
         // Set custom CA file if provided
         if (tls_options.CustomCAFile().has_value()) {
-            CURL_SETOPT_CHECK(curl, CURLOPT_CAINFO, tls_options.CustomCAFile()->c_str());
+            CURL_SETOPT_CHECK(curl.get(), CURLOPT_CAINFO, tls_options.CustomCAFile()->c_str());
         }
     }
 
@@ -216,19 +207,19 @@ void CurlRequester::PerformRequestWithMulti(std::shared_ptr<CurlMultiManager> mu
     // Empty string explicitly disables proxy (overrides environment variables).
     auto const& proxy_url = request.Properties().Proxy().Url();
     if (proxy_url.has_value()) {
-        CURL_SETOPT_CHECK(curl, CURLOPT_PROXY, proxy_url->c_str());
+        CURL_SETOPT_CHECK(curl.get(), CURLOPT_PROXY, proxy_url->c_str());
     }
     // If proxy URL is std::nullopt, CURL will use environment variables (default behavior)
 
     // Set callbacks
-    CURL_SETOPT_CHECK(curl, CURLOPT_WRITEFUNCTION, WriteCallback);
-    CURL_SETOPT_CHECK(curl, CURLOPT_WRITEDATA, &ctx->response_body);
-    CURL_SETOPT_CHECK(curl, CURLOPT_HEADERFUNCTION, HeaderCallback);
-    CURL_SETOPT_CHECK(curl, CURLOPT_HEADERDATA, &ctx->response_headers);
+    CURL_SETOPT_CHECK(curl.get(), CURLOPT_WRITEFUNCTION, WriteCallback);
+    CURL_SETOPT_CHECK(curl.get(), CURLOPT_WRITEDATA, &ctx->response_body);
+    CURL_SETOPT_CHECK(curl.get(), CURLOPT_HEADERFUNCTION, HeaderCallback);
+    CURL_SETOPT_CHECK(curl.get(), CURLOPT_HEADERDATA, &ctx->response_headers);
 
     // Follow redirects
-    CURL_SETOPT_CHECK(curl, CURLOPT_FOLLOWLOCATION, 1L);
-    CURL_SETOPT_CHECK(curl, CURLOPT_MAXREDIRS, 20L);
+    CURL_SETOPT_CHECK(curl.get(), CURLOPT_FOLLOWLOCATION, 1L);
+    CURL_SETOPT_CHECK(curl.get(), CURLOPT_MAXREDIRS, 20L);
 
     #undef CURL_SETOPT_CHECK
 

libs/networking/include/launchdarkly/network/curl_multi_manager.hpp

@@ -60,7 +60,7 @@ class CurlMultiManager : public std::enable_shared_from_this<CurlMultiManager> {
      * @param headers The curl_slist headers (will be freed automatically)
      * @param callback Called when the transfer completes
      */
-    void add_handle(CURL* easy,
+    void add_handle(std::shared_ptr<CURL> easy,
                     curl_slist* headers,
                     CompletionCallback callback);
 
@@ -112,6 +112,7 @@ class CurlMultiManager : public std::enable_shared_from_this<CurlMultiManager> {
     std::mutex mutex_;
     std::map<CURL*, CompletionCallback> callbacks_;
     std::map<CURL*, curl_slist*> headers_;
+    std::map<CURL*, std::shared_ptr<CURL>> handles_;
     std::map<curl_socket_t, SocketInfo> sockets_; // Managed socket info
     int still_running_{0};
 };

libs/networking/src/curl_multi_manager.cpp

@@ -43,49 +43,49 @@ CurlMultiManager::~CurlMultiManager() {
                 curl_slist_free_all(header_it->second);
             }
 
-            curl_easy_cleanup(easy);
+            // curl_easy_cleanup(easy);
         }
     }
 }
 
-void CurlMultiManager::add_handle(CURL* easy,
+void CurlMultiManager::add_handle(std::shared_ptr<CURL> easy,
                                   curl_slist* headers,
                                   CompletionCallback callback) {
-    {
-        std::lock_guard lock(mutex_);
-        callbacks_[easy] = std::move(callback);
-        headers_[easy] = headers;
-    }
-
-    if (const CURLMcode rc = curl_multi_add_handle(multi_handle_.get(), easy);
+    if (const CURLMcode rc = curl_multi_add_handle(
+            multi_handle_.get(), easy.get());
         rc != CURLM_OK) {
-        std::lock_guard lock(mutex_);
-        callbacks_.erase(easy);
-
         // Free headers on error
-        if (const auto header_it = headers_.find(easy);
-            header_it != headers_.end() && header_it->second) {
-            curl_slist_free_all(header_it->second);
+        if (headers) {
+            curl_slist_free_all(headers);
         }
-        headers_.erase(easy);
 
         std::cerr << "Failed to add handle to multi: "
             << curl_multi_strerror(rc) << std::endl;
+        return;
+    }
+
+    {
+        std::lock_guard lock(mutex_);
+        callbacks_[easy.get()] = std::move(callback);
+        headers_[easy.get()] = headers;
+        handles_[easy.get()] = easy;
     }
 }
 
 void CurlMultiManager::remove_handle(CURL* easy) {
     curl_multi_remove_handle(multi_handle_.get(), easy);
 
     std::lock_guard lock(mutex_);
-    callbacks_.erase(easy);
 
     // Free headers if they exist
     if (const auto header_it = headers_.find(easy);
         header_it != headers_.end() && header_it->second) {
         curl_slist_free_all(header_it->second);
     }
+
+    callbacks_.erase(easy);
     headers_.erase(easy);
+    handles_.erase(easy);
 }
 
 int CurlMultiManager::socket_callback(CURL* easy,
@@ -182,17 +182,21 @@ void CurlMultiManager::check_multi_info() {
             curl_slist* headers = nullptr;
             {
                 std::lock_guard lock(mutex_);
+
                 if (auto it = callbacks_.find(easy); it != callbacks_.end()) {
                     callback = std::move(it->second);
                     callbacks_.erase(it);
                 }
+                callbacks_.erase(easy);
 
                 // Get and remove headers
                 if (auto header_it = headers_.find(easy);
                     header_it != headers_.end()) {
                     headers = header_it->second;
                     headers_.erase(header_it);
                 }
+
+                handles_.erase(easy);
             }
 
             // Remove from multi handle
@@ -224,10 +228,12 @@ void CurlMultiManager::start_socket_monitor(SocketInfo* socket_info,
         // Assign the CURL socket to the ASIO socket
         // tcp::socket::assign works with native socket handles on both platforms
         boost::system::error_code ec;
-        socket_info->handle->assign(boost::asio::ip::tcp::v4(), socket_info->sockfd, ec);
+        socket_info->handle->assign(boost::asio::ip::tcp::v4(),
+                                    socket_info->sockfd, ec);
 
         if (ec) {
-            std::cerr << "Failed to assign socket: " << ec.message() << std::endl;
+            std::cerr << "Failed to assign socket: " << ec.message() <<
+                std::endl;
             socket_info->handle.reset();
             return;
         }
@@ -249,36 +255,41 @@ void CurlMultiManager::start_socket_monitor(SocketInfo* socket_info,
 
             // Create and store handler in SocketInfo to keep it alive
             // Use weak_ptr in capture to avoid circular reference
-            socket_info->read_handler = std::make_shared<std::function<void()>>();
-            std::weak_ptr<std::function<void()>> weak_read_handler = socket_info->read_handler;
-            *socket_info->read_handler = [weak_self, sockfd, weak_handle, weak_read_handler]() {
-                // Check if manager and handle are still valid
-                const auto self = weak_self.lock();
-                const auto handle = weak_handle.lock();
-                if (!self || !handle) {
-                    return;
-                }
+            socket_info->read_handler = std::make_shared<std::function<void
+                ()>>();
+            std::weak_ptr<std::function<void()>> weak_read_handler = socket_info
+                ->read_handler;
+            *socket_info->read_handler = [weak_self, sockfd, weak_handle,
+                    weak_read_handler]() {
+                    // Check if manager and handle are still valid
+                    const auto self = weak_self.lock();
+                    const auto handle = weak_handle.lock();
+                    if (!self || !handle) {
+                        return;
+                    }
 
-                handle->async_wait(
-                    boost::asio::ip::tcp::socket::wait_read,
-                    [weak_self, sockfd, weak_handle, weak_read_handler](
+                    handle->async_wait(
+                        boost::asio::ip::tcp::socket::wait_read,
+                        [weak_self, sockfd, weak_handle, weak_read_handler](
                         const boost::system::error_code& ec) {
-                        // If operation was canceled or had an error, don't re-register
-                        if (ec) {
-                            return;
-                        }
-
-                        if (const auto self = weak_self.lock()) {
-                            self->handle_socket_action(sockfd, CURL_CSELECT_IN);
-
-                            // Always try to re-register for continuous monitoring
-                            // The validity check at the top of read_handler will stop it if needed
-                            if (const auto handler = weak_read_handler.lock()) {
-                                (*handler)(); // Recursive call
+                            // If operation was canceled or had an error, don't re-register
+                            if (ec) {
+                                return;
+                            }
+
+                            if (const auto self = weak_self.lock()) {
+                                self->handle_socket_action(
+                                    sockfd, CURL_CSELECT_IN);
+
+                                // Always try to re-register for continuous monitoring
+                                // The validity check at the top of read_handler will stop it if needed
+                                if (const auto handler = weak_read_handler.
+                                    lock()) {
+                                    (*handler)(); // Recursive call
+                                }
                             }
-                        }
-                    });
-            };
+                        });
+                };
             (*socket_info->read_handler)(); // Initial call
         }
     }
@@ -292,36 +303,41 @@ void CurlMultiManager::start_socket_monitor(SocketInfo* socket_info,
 
             // Create and store handler in SocketInfo to keep it alive
             // Use weak_ptr in capture to avoid circular reference
-            socket_info->write_handler = std::make_shared<std::function<void()>>();
-            std::weak_ptr<std::function<void()>> weak_write_handler = socket_info->write_handler;
-            *socket_info->write_handler = [weak_self, sockfd, weak_handle, weak_write_handler]() {
-                // Check if manager and handle are still valid
-                const auto self = weak_self.lock();
-                const auto handle = weak_handle.lock();
-                if (!self || !handle) {
-                    return;
-                }
+            socket_info->write_handler = std::make_shared<std::function<void
+                ()>>();
+            std::weak_ptr<std::function<void()>> weak_write_handler =
+                socket_info->write_handler;
+            *socket_info->write_handler = [weak_self, sockfd, weak_handle,
+                    weak_write_handler]() {
+                    // Check if manager and handle are still valid
+                    const auto self = weak_self.lock();
+                    const auto handle = weak_handle.lock();
+                    if (!self || !handle) {
+                        return;
+                    }
 
-                handle->async_wait(
-                    boost::asio::ip::tcp::socket::wait_write,
-                    [weak_self, sockfd, weak_handle, weak_write_handler](
+                    handle->async_wait(
+                        boost::asio::ip::tcp::socket::wait_write,
+                        [weak_self, sockfd, weak_handle, weak_write_handler](
                         const boost::system::error_code& ec) {
-                        // If operation was canceled or had an error, don't re-register
-                        if (ec) {
-                            return;
-                        }
-
-                        if (const auto self = weak_self.lock()) {
-                            self->handle_socket_action(sockfd, CURL_CSELECT_OUT);
-
-                            // Always try to re-register for continuous monitoring
-                            // The validity check at the top of write_handler will stop it if needed
-                            if (const auto handler = weak_write_handler.lock()) {
-                                (*handler)(); // Recursive call
+                            // If operation was canceled or had an error, don't re-register
+                            if (ec) {
+                                return;
+                            }
+
+                            if (const auto self = weak_self.lock()) {
+                                self->handle_socket_action(
+                                    sockfd, CURL_CSELECT_OUT);
+
+                                // Always try to re-register for continuous monitoring
+                                // The validity check at the top of write_handler will stop it if needed
+                                if (const auto handler = weak_write_handler.
+                                    lock()) {
+                                    (*handler)(); // Recursive call
+                                }
                             }
-                        }
-                    });
-            };
+                        });
+                };
             (*socket_info->write_handler)(); // Initial call
         }
     }