Remove debug logging.

Author: kinyoklion

libs/networking/src/curl_multi_manager.cpp

@@ -4,8 +4,6 @@
 
 #include <boost/asio/post.hpp>
 
-#include <iostream>
-
 namespace launchdarkly::network {
 std::shared_ptr<CurlMultiManager> CurlMultiManager::create(
     boost::asio::any_io_executor executor) {
@@ -49,7 +47,8 @@ CurlMultiManager::~CurlMultiManager() {
 void CurlMultiManager::add_handle(const std::shared_ptr<CURL>& easy,
                                   curl_slist* headers,
                                   CompletionCallback callback,
-                                  std::optional<std::chrono::milliseconds> read_timeout) {
+                                  std::optional<std::chrono::milliseconds>
+                                  read_timeout) {
     if (const CURLMcode rc = curl_multi_add_handle(
             multi_handle_.get(), easy.get());
         rc != CURLM_OK) {
@@ -58,8 +57,6 @@ void CurlMultiManager::add_handle(const std::shared_ptr<CURL>& easy,
             curl_slist_free_all(headers);
         }
 
-        std::cerr << "Failed to add handle to multi: "
-            << curl_multi_strerror(rc) << std::endl;
         return;
     }
 
@@ -72,19 +69,21 @@ void CurlMultiManager::add_handle(const std::shared_ptr<CURL>& easy,
         // Setup read timeout timer if specified
         if (read_timeout) {
             auto timer = std::make_shared<boost::asio::steady_timer>(executor_);
-            handle_timeouts_[easy.get()] = HandleTimeoutInfo{read_timeout, timer};
+            handle_timeouts_[easy.get()] = HandleTimeoutInfo{
+                read_timeout, timer};
 
             // Start the timeout timer
             timer->expires_after(*read_timeout);
             auto weak_self = weak_from_this();
             CURL* easy_ptr = easy.get();
-            timer->async_wait([weak_self, easy_ptr](const boost::system::error_code& ec) {
-                if (!ec) {
-                    if (auto self = weak_self.lock()) {
-                        self->handle_read_timeout(easy_ptr);
+            timer->async_wait(
+                [weak_self, easy_ptr](const boost::system::error_code& ec) {
+                    if (!ec) {
+                        if (auto self = weak_self.lock()) {
+                            self->handle_read_timeout(easy_ptr);
+                        }
                     }
-                }
-            });
+                });
         }
     }
 }
@@ -153,14 +152,11 @@ int CurlMultiManager::timer_callback(CURLM* multi,
 void CurlMultiManager::handle_socket_action(curl_socket_t s,
                                             const int event_bitmask) {
     int running_handles = 0;
-    const CURLMcode rc = curl_multi_socket_action(multi_handle_.get(), s,
-                                                  event_bitmask,
-                                                  &running_handles);
-
-    if (rc != CURLM_OK) {
-        std::cerr << "curl_multi_socket_action failed: "
-            << curl_multi_strerror(rc) << std::endl;
-    }
+    // This can return an error code, but checking the multi_info will be
+    // sufficient without additional handling for this error code.
+    curl_multi_socket_action(multi_handle_.get(), s,
+                             event_bitmask,
+                             &running_handles);
 
     check_multi_info();
 
@@ -191,7 +187,6 @@ void CurlMultiManager::check_multi_info() {
                     callback = std::move(it->second);
                     callbacks_.erase(it);
                 }
-                callbacks_.erase(easy);
 
                 // Get and remove headers
                 if (auto header_it = headers_.find(easy);
@@ -226,7 +221,8 @@ void CurlMultiManager::check_multi_info() {
             if (callback) {
                 boost::asio::post(executor_, [callback = std::move(callback),
                                       result, handle]() {
-                                      callback(handle, Result::FromCurlCode(result));
+                                      callback(
+                                          handle, Result::FromCurlCode(result));
                                   });
             }
         }
@@ -247,8 +243,6 @@ void CurlMultiManager::start_socket_monitor(SocketInfo* socket_info,
                                     socket_info->sockfd, ec);
 
         if (ec) {
-            std::cerr << "Failed to assign socket: " << ec.message() <<
-                std::endl;
             socket_info->handle.reset();
             return;
         }
@@ -368,13 +362,14 @@ void CurlMultiManager::reset_read_timeout(CURL* easy) {
 
         auto weak_self = weak_from_this();
         CURL* easy_ptr = easy;
-        timeout_info.timer->async_wait([weak_self, easy_ptr](const boost::system::error_code& ec) {
-            if (!ec) {
-                if (auto self = weak_self.lock()) {
-                    self->handle_read_timeout(easy_ptr);
+        timeout_info.timer->async_wait(
+            [weak_self, easy_ptr](const boost::system::error_code& ec) {
+                if (!ec) {
+                    if (auto self = weak_self.lock()) {
+                        self->handle_read_timeout(easy_ptr);
+                    }
                 }
-            }
-        });
+            });
     }
 }
 
@@ -389,7 +384,7 @@ void CurlMultiManager::handle_read_timeout(CURL* easy) {
         // Check if handle still exists
         auto it = callbacks_.find(easy);
         if (it == callbacks_.end()) {
-            return;  // Handle already completed
+            return; // Handle already completed
         }
 
         // Get and remove callback
@@ -429,9 +424,9 @@ void CurlMultiManager::handle_read_timeout(CURL* easy) {
     // Invoke completion callback with read timeout result
     if (callback) {
         boost::asio::post(executor_, [callback = std::move(callback),
-                                      handle]() {
-                                      callback(handle, Result::FromReadTimeout());
-                                  });
+                              handle]() {
+                              callback(handle, Result::FromReadTimeout());
+                          });
     }
 }
 } // namespace launchdarkly::network

libs/server-sent-events/src/curl_client.hpp

@@ -27,20 +27,22 @@ namespace net = boost::asio;
 using launchdarkly::network::CurlMultiManager;
 
 /**
- * The lifecycle of the CurlClient is managed in an RAII manner. This introduces
- * some complexity with interaction with CURL, which requires a thread to be
- * driven. The desutruction of the CurlClient will signal that the CURL thread
- * should stop. Though depending on the operation it may linger for some
- * time.
- *
- * The CurlClient itself is not accessible from the CURL thread and instead
- * all their shared state is in a RequestContext. The lifetime of this context
- * can exceed that of the CurlClient while CURL shuts down. This approach
- * prevents the lifetime of the CurlClient being attached to that of the
- * curl thread.
+ * The CurlClient uses the CURL multi-socket interface to allow for
+ * single-threaded usage of CURL. We drive this usage using boost::asio
+ * and every thing is executed in the IO context provided during client
+ * construction. Calling into the API of the client could be done from threads
+ * other than the IO context thread, so some thread-safety is required to
+ * manage those interactions. For example the CurlClient destructor will
+ * be ran on whatever thread last retains a reference to the client.
  */
 class CurlClient final : public Client,
                          public std::enable_shared_from_this<CurlClient> {
+    /**
+    * Structure containing callbacks between the CURL interactions and the
+    * IO executor. Callbacks are set while a connection is being established,
+    * instead of at construction time, to allow the use of weak_from_self.
+    * The weak_from_self method cannot be used during the constructor.
+    */
     struct Callbacks {
         std::function<void(std::string)> do_backoff;
         std::function<void(Event)> on_receive;
@@ -63,6 +65,16 @@ class CurlClient final : public Client,
         }
     };
 
+    /**
+    * The request context represents the state required by the executing CURL
+    * request. Not directly including the shared data in the CurlClient allows
+    * for easy seperation of its lifetime from that of the CURL client. This
+    * facilitates destruction of the CurlClient being used to stop in-progress
+    * requests.
+    *
+    * Also the CURL client can be destructed and pending tasks will still
+    * have a valid RequestContext and will detect the shutdown.
+    */
     class RequestContext {
         // Only items used by both the curl thread and the executor/main
         // thread need to be mutex protected.