Tweak CurlHttpclient to match style

Author: MatthewColvin

Platformio/lib/SimulatorHalImpl/wifiHandlerSim/CurlHttpClient.cpp

@@ -4,45 +4,55 @@
 #include <sstream>
 
 // Callback for curl to write response data
-static size_t WriteCallback(void *contents, size_t size, size_t nmemb, std::string *userp) {
-  userp->append((char *)contents, size * nmemb);
-  return size * nmemb;
+static size_t WriteCallback(char *contents, size_t size, size_t nmemb, void *userp) {
+  size_t realsize = size * nmemb;
+  if (userp) {
+    std::string *str = static_cast<std::string *>(userp);
+    str->append(contents, realsize);
+  }
+  return realsize;
 }
 
 // Callback for curl to write response headers
-static size_t HeaderCallback(char *buffer, size_t size, size_t nmemb, std::map<std::string, std::string> *userp) {
+static size_t HeaderCallback(char *buffer, size_t size, size_t nmemb, void *userp) {
   size_t realsize = size * nmemb;
   std::string header(buffer, realsize);
 
   // Parse header line (format: "Header-Name: value\r\n")
   size_t colon_pos = header.find(':');
-  if (colon_pos != std::string::npos) {
+  if (colon_pos != std::string::npos && userp) {
     std::string key = header.substr(0, colon_pos);
-    std::string value = header.substr(colon_pos + 2);
+    // allow for ": " after key but be defensive
+    size_t value_start = colon_pos + 1;
+    if (value_start < header.size() && header[value_start] == ' ') {
+      ++value_start;
+    }
+    std::string value = header.substr(value_start);
 
     // Trim trailing whitespace/CRLF
     while (!value.empty() && (value.back() == '\r' || value.back() == '\n' || value.back() == ' ')) {
       value.pop_back();
     }
 
-    userp->emplace(key, value);
+    auto *mapPtr = static_cast<std::map<std::string, std::string> *>(userp);
+    mapPtr->emplace(key, value);
   }
 
   return realsize;
 }
 
 CurlHttpClient::CurlHttpClient(size_t num_threads)
-    : shutdown_requested_(false), initialized_(false) {
+    : mShutdownRequested(false), mInitialized(false) {
 
   // Initialize curl globally
   curl_global_init(CURL_GLOBAL_DEFAULT);
 
   // Start worker threads
   for (size_t i = 0; i < num_threads; ++i) {
-    worker_threads_.emplace_back(&CurlHttpClient::workerThread, this);
+    mWorkerThreads.emplace_back(&CurlHttpClient::workerThread, this);
   }
 
-  initialized_ = true;
+  mInitialized = true;
 }
 
 CurlHttpClient::~CurlHttpClient() {
@@ -52,17 +62,12 @@ CurlHttpClient::~CurlHttpClient() {
 std::shared_ptr<HttpFuture> CurlHttpClient::executeAsync(const HttpRequest &request) {
   auto promise = std::make_shared<std::promise<HttpResponse>>();
   auto future = promise->get_future();
-
   {
-    std::lock_guard<std::mutex> lock(queue_mutex_);
-    request_queue_.push(request);
-    promise_queue_.push(std::move(*promise));
+    std::lock_guard<std::mutex> lock(mQueueMutex);
+    mRequestQueue.emplace(request, promise);
   }
-
-  queue_cv_.notify_one();
-
-  auto http_future = std::make_shared<HttpFuture>(std::move(future));
-  return http_future;
+  mQueueCv.notify_one();
+  return std::make_shared<HttpFuture>(std::move(future));
 }
 
 std::shared_ptr<HttpFuture> CurlHttpClient::getAsync(const std::string &url, int timeout_ms) {
@@ -98,33 +103,33 @@ std::shared_ptr<HttpFuture> CurlHttpClient::deleteAsync(
 }
 
 void CurlHttpClient::setDefaultHeader(const std::string &key, const std::string &value) {
-  std::lock_guard<std::mutex> lock(headers_mutex_);
+  std::lock_guard<std::mutex> lock(mHeadersMutex);
   default_headers_[key] = value;
 }
 
 void CurlHttpClient::clearDefaultHeaders() {
-  std::lock_guard<std::mutex> lock(headers_mutex_);
+  std::lock_guard<std::mutex> lock(mHeadersMutex);
   default_headers_.clear();
 }
 
 bool CurlHttpClient::isReady() const {
-  return initialized_ && !shutdown_requested_;
+  return mInitialized && !mShutdownRequested;
 }
 
 bool CurlHttpClient::initialize() {
   // Already initialized in constructor
-  return initialized_;
+  return mInitialized;
 }
 
 void CurlHttpClient::shutdown() {
   {
-    std::lock_guard<std::mutex> lock(queue_mutex_);
-    shutdown_requested_ = true;
+    std::lock_guard<std::mutex> lock(mQueueMutex);
+    mShutdownRequested = true;
   }
-  queue_cv_.notify_all();
+  mQueueCv.notify_all();
 
   // Wait for all worker threads to finish
-  for (auto &thread : worker_threads_) {
+  for (auto &thread : mWorkerThreads) {
     if (thread.joinable()) {
       thread.join();
     }
@@ -135,39 +140,22 @@ void CurlHttpClient::shutdown() {
 
 void CurlHttpClient::workerThread() {
   while (true) {
-    std::unique_lock<std::mutex> lock(queue_mutex_);
-
+    std::unique_lock<std::mutex> lock(mQueueMutex);
     // Wait for a request or shutdown signal
-    queue_cv_.wait(lock, [this]() {
-      return !request_queue_.empty() || shutdown_requested_;
+    mQueueCv.wait(lock, [this]() {
+      return !mRequestQueue.empty() || mShutdownRequested;
     });
-
-    if (shutdown_requested_ && request_queue_.empty()) {
+    if (mShutdownRequested && mRequestQueue.empty()) {
       break;
     }
-
-    if (request_queue_.empty()) {
-      continue;
-    }
-
-    // Get the next request and promise pair
-    HttpRequest request = std::move(request_queue_.front());
-    request_queue_.pop();
-    std::promise<HttpResponse> promise = std::move(promise_queue_.front());
-    promise_queue_.pop();
+    // Get the request to process
+    auto [request, promise] = mRequestQueue.front();
+    mRequestQueue.pop();
     lock.unlock();
 
-    // Execute the request
-    try {
-      HttpResponse response = executeSyncRequest(request);
-      promise.set_value(response);
-    } catch (const std::exception &e) {
-      HttpResponse error_response(
-          -1,
-          "",
-          std::string("Error: ") + e.what());
-      error_response.success = false;
-      promise.set_value(error_response);
+    HttpResponse response = executeSyncRequest(request);
+    if (auto sPromise = promise.lock(); sPromise) {
+      sPromise->set_value(response);
     }
   }
 }
@@ -188,8 +176,12 @@ HttpResponse CurlHttpClient::executeSyncRequest(const HttpRequest &request) {
     // Set URL
     curl_easy_setopt(curl, CURLOPT_URL, request.url.c_str());
 
-    // Set timeout
+    // Set timeouts (connection and total transfer)
+    curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT_MS, (long)request.timeout_ms);
     curl_easy_setopt(curl, CURLOPT_TIMEOUT_MS, (long)request.timeout_ms);
+    // In multithreaded programs, libcurl should not install or use signals.
+    // Prevent use of signals (like SIGALRM) which can interfere with threads.
+    curl_easy_setopt(curl, CURLOPT_NOSIGNAL, 1L);
 
     // Set HTTP method
     switch (request.method) {

Platformio/lib/SimulatorHalImpl/wifiHandlerSim/CurlHttpClient.hpp

@@ -10,22 +10,15 @@
 /**
  * @brief Curl-based HTTP client for desktop simulator
  *
- * This implementation uses libcurl to provide HTTP functionality
- * on desktop platforms. Requests are executed asynchronously using
- * a thread pool pattern with custom HttpFuture for fluent callbacks.
+ * Uses a thread pool and queue to handle asynchronous HTTP requests
+ * via libcurl. Implements HttpClientInterface with custom HttpFuture
+ * for fluent callback handling.
  */
 class CurlHttpClient : public HttpClientInterface {
 public:
-  /**
-   * @brief Constructor
-   *
-   * @param num_threads Number of worker threads for async requests (default: 2)
-   */
-  explicit CurlHttpClient(size_t num_threads = 2);
+  using QueueRequestItemType = std::pair<HttpRequest, std::weak_ptr<std::promise<HttpResponse>>>;
 
-  /**
-   * @brief Destructor - ensures proper cleanup
-   */
+  explicit CurlHttpClient(size_t aNumWorkerThreads = 2);
   ~CurlHttpClient() override;
 
   // Prevent copy operations
@@ -80,18 +73,19 @@ class CurlHttpClient : public HttpClientInterface {
    */
   static std::string methodToString(HttpRequest::Method method);
 
-  // Thread pool management
-  std::vector<std::thread> worker_threads_;
-  std::queue<HttpRequest> request_queue_;
-  std::queue<std::promise<HttpResponse>> promise_queue_;
-  mutable std::mutex queue_mutex_;
-  std::condition_variable queue_cv_;
-  bool shutdown_requested_ = false;
+  // Thread pool for executing requests
+  std::vector<std::thread> mWorkerThreads;
+
+  // Queue for thread-safe non-blocking request handling
+  mutable std::mutex mQueueMutex;
+  std::condition_variable mQueueCv;
+  std::queue<QueueRequestItemType> mRequestQueue;
 
   // Default headers
   std::map<std::string, std::string> default_headers_;
-  mutable std::mutex headers_mutex_;
+  mutable std::mutex mHeadersMutex;
 
   // State
-  bool initialized_ = false;
+  bool mInitialized = false;
+  bool mShutdownRequested = false;
 };