CCDBApi: avoid leaking curl handle.

CCDB/include/CCDB/CCDBDownloader.h

@@ -52,6 +52,7 @@ typedef struct DownloaderRequestData {
   HeaderObjectPair_t hoPair;
   std::map<std::string, std::string>* headers;
   std::string userAgent;
+  curl_slist* optionsList;
 
   std::function<bool(std::string)> localContentCallback;
 } DownloaderRequestData;
@@ -296,6 +297,7 @@ class CCDBDownloader
     int hostInd;
     int locInd;
     DownloaderRequestData* requestData;
+    curl_slist** options;
   } PerformData;
 #endif
 
@@ -421,6 +423,6 @@ typedef struct DataForClosingSocket {
   curl_socket_t socket;
 } DataForClosingSocket;
 
-} // namespace o2
+} // namespace o2::ccdb
 
 #endif // O2_CCDB_CCDBDOWNLOADER_H

CCDB/src/CCDBDownloader.cxx

@@ -535,8 +535,10 @@ void CCDBDownloader::transferFinished(CURL* easy_handle, CURLcode curlCode)
           }
         }
         --(*performData->requestsLeft);
+        curl_slist_free_all(*performData->options);
         delete requestData;
         delete performData->codeDestination;
+        curl_easy_cleanup(easy_handle);
       }
     } break;
   }
@@ -681,6 +683,7 @@ void CCDBDownloader::asynchSchedule(CURL* handle, size_t* requestCounter)
   curl_easy_getinfo(handle, CURLINFO_PRIVATE, &requestData);
   headerMap = &(requestData->hoPair.header);
   hostsPool = &(requestData->hosts);
+  auto* options = &(requestData->optionsList);
 
   // Prepare temporary data about transfer
   auto* data = new CCDBDownloader::PerformData(); // Freed in transferFinished
@@ -692,6 +695,7 @@ void CCDBDownloader::asynchSchedule(CURL* handle, size_t* requestCounter)
   data->hostInd = 0;
   data->locInd = 0;
   data->requestData = requestData;
+  data->options = options;
 
   // Prepare handle and schedule download
   setHandleOptions(handle, data);
@@ -719,4 +723,4 @@ std::string CCDBDownloader::prepareLogMessage(std::string host_url, std::string
   return fmt::format("CcdbDownloader finished transfer {}{}{} for {} (agent_id: {}) with http code: {}", host_url, (host_url.back() == '/') ? "" : "/", upath, (ts < 0) ? getCurrentTimestamp() : ts, userAgent, httpCode);
 }
 
-} // namespace o2
+} // namespace o2::ccdb

CCDB/src/CcdbApi.cxx

@@ -1656,6 +1656,7 @@ void CcdbApi::scheduleDownload(RequestContext& requestContext, size_t* requestCo
   data->timestamp = requestContext.timestamp;
   data->localContentCallback = localContentCallback;
   data->userAgent = mUniqueAgentID;
+  data->optionsList = options_list;
 
   curl_easy_setopt(curl_handle, CURLOPT_URL, fullUrl.c_str());
   initCurlOptionsForRetrieve(curl_handle, (void*)(&data->hoPair), writeCallback, false);

CCDB/test/testCcdbApiDownloader.cxx

@@ -23,7 +23,6 @@
 #include <fairlogger/Logger.h>
 
 #include <boost/test/unit_test.hpp>
-#include <boost/optional/optional.hpp>
 #include <boost/asio/ip/host_name.hpp>
 #include <uv.h>
 
@@ -129,7 +128,7 @@ std::vector<CURL*> prepareAsyncHandles(size_t num, std::vector<o2::pmr::vector<c
 
     auto data = new DownloaderRequestData();
     data->hoPair.object = dest;
-    data->hosts.push_back("http://ccdb-test.cern.ch:8080");
+    data->hosts.emplace_back("http://ccdb-test.cern.ch:8080");
     data->path = "Analysis/ALICE3/Centrality";
     data->timestamp = 1646729604010;
     data->localContentCallback = nullptr;
@@ -164,15 +163,17 @@ BOOST_AUTO_TEST_CASE(asynch_schedule_test)
   }
 
   while (transfersLeft > 0) {
-    downloader.runLoop(0);
+    downloader.runLoop(false);
   }
 
   for (int i = 0; i < TRANSFERS; i++) {
     long httpCode;
     curl_easy_getinfo(handles[i], CURLINFO_HTTP_CODE, &httpCode);
-    BOOST_CHECK(httpCode == 200);
-    BOOST_CHECK(dests[i]->size() != 0);
-    curl_easy_cleanup(handles[i]);
+    BOOST_CHECK_EQUAL(httpCode,  200);
+    BOOST_CHECK_NE(dests[i]->size(), 0);
+    // I suspect the following was needed simply because the rest of the
+    // code was leaking.
+    //    curl_easy_cleanup(handles[i]);
     delete dests[i];
   }
   curl_global_cleanup();
@@ -191,13 +192,11 @@ BOOST_AUTO_TEST_CASE(perform_test)
 
   CURLcode curlCode = downloader.perform(handle);
 
-  BOOST_CHECK(curlCode == CURLE_OK);
-  std::cout << "CURL code: " << curlCode << "\n";
+  BOOST_CHECK_EQUAL(curlCode, CURLE_OK);
 
   long httpCode;
   curl_easy_getinfo(handle, CURLINFO_HTTP_CODE, &httpCode);
-  BOOST_CHECK(httpCode == 200);
-  std::cout << "HTTP code: " << httpCode << "\n";
+  BOOST_CHECK_EQUAL(httpCode, 200);
 
   curl_easy_cleanup(handle);
   curl_global_cleanup();
@@ -220,19 +219,13 @@ BOOST_AUTO_TEST_CASE(blocking_batch_test)
 
   auto curlCodes = downloader.batchBlockingPerform(handleVector);
   for (CURLcode code : curlCodes) {
-    BOOST_CHECK(code == CURLE_OK);
-    if (code != CURLE_OK) {
-      std::cout << "CURL Code: " << code << "\n";
-    }
+    BOOST_CHECK_EQUAL(code, CURLE_OK);
   }
 
   for (CURL* handle : handleVector) {
     long httpCode;
     curl_easy_getinfo(handle, CURLINFO_HTTP_CODE, &httpCode);
-    BOOST_CHECK(httpCode == 200);
-    if (httpCode != 200) {
-      std::cout << "HTTP Code: " << httpCode << "\n";
-    }
+    BOOST_CHECK_EQUAL(httpCode, 200);
     curl_easy_cleanup(handle);
   }
 
@@ -261,19 +254,13 @@ BOOST_AUTO_TEST_CASE(test_with_break)
   auto curlCodes = downloader.batchBlockingPerform(handleVector);
 
   for (CURLcode code : curlCodes) {
-    BOOST_CHECK(code == CURLE_OK);
-    if (code != CURLE_OK) {
-      std::cout << "CURL Code: " << code << "\n";
-    }
+    BOOST_CHECK_EQUAL(code, CURLE_OK);
   }
 
   for (CURL* handle : handleVector) {
     long httpCode;
     curl_easy_getinfo(handle, CURLINFO_HTTP_CODE, &httpCode);
-    BOOST_CHECK(httpCode == 200);
-    if (httpCode != 200) {
-      std::cout << "HTTP Code: " << httpCode << "\n";
-    }
+    BOOST_CHECK_EQUAL(httpCode, 200);
     curl_easy_cleanup(handle);
   }
 
@@ -292,19 +279,13 @@ BOOST_AUTO_TEST_CASE(test_with_break)
 
   auto curlCodes2 = downloader.batchBlockingPerform(handleVector2);
   for (CURLcode code : curlCodes2) {
-    BOOST_CHECK(code == CURLE_OK);
-    if (code != CURLE_OK) {
-      std::cout << "CURL Code: " << code << "\n";
-    }
+    BOOST_CHECK_EQUAL(code, CURLE_OK);
   }
 
   for (CURL* handle : handleVector2) {
     long httpCode;
     curl_easy_getinfo(handle, CURLINFO_HTTP_CODE, &httpCode);
-    BOOST_CHECK(httpCode == 200);
-    if (httpCode != 200) {
-      std::cout << "HTTP Code: " << httpCode << "\n";
-    }
+    BOOST_CHECK_EQUAL(httpCode, 200);
     curl_easy_cleanup(handle);
   }
 
@@ -357,18 +338,18 @@ BOOST_AUTO_TEST_CASE(external_loop_test)
 
   CURLcode curlCode = downloader->perform(handle);
 
-  BOOST_CHECK(curlCode == CURLE_OK);
+  BOOST_CHECK_EQUAL(curlCode, CURLE_OK);
 
   long httpCode;
   curl_easy_getinfo(handle, CURLINFO_HTTP_CODE, &httpCode);
-  BOOST_CHECK(httpCode == 200);
+  BOOST_CHECK_EQUAL(httpCode, 200);
 
   curl_easy_cleanup(handle);
   curl_global_cleanup();
 
   // Check if test timer and external loop are still alive
-  BOOST_CHECK(uv_is_active((uv_handle_t*)testTimer) != 0);
-  BOOST_CHECK(uv_loop_alive(uvLoop) != 0);
+  BOOST_CHECK_NE(uv_is_active((uv_handle_t*)testTimer), 0);
+  BOOST_CHECK_NE(uv_loop_alive(uvLoop), 0);
 
   // Downloader must be closed before uv_loop.
   // The reason for that are the uv_poll handles attached to the curl multi handle.
@@ -384,11 +365,11 @@ BOOST_AUTO_TEST_CASE(external_loop_test)
 BOOST_AUTO_TEST_CASE(trim_host_url_test)
 {
   CCDBDownloader downloader;
-  BOOST_CHECK(downloader.trimHostUrl("http://localhost:8080") == "http://localhost:8080");
-  BOOST_CHECK(downloader.trimHostUrl("http://localhost") == "http://localhost");
-  BOOST_CHECK(downloader.trimHostUrl("http://localhost:8080/some/path") == "http://localhost:8080");
-  BOOST_CHECK(downloader.trimHostUrl("http://localhost/some/path") == "http://localhost");
-  BOOST_CHECK(downloader.trimHostUrl("http://localhost:8080/Task/Detector/1?HTTPOnly=true") == "http://localhost:8080");
+  BOOST_CHECK_EQUAL(downloader.trimHostUrl("http://localhost:8080"), "http://localhost:8080");
+  BOOST_CHECK_EQUAL(downloader.trimHostUrl("http://localhost"), "http://localhost");
+  BOOST_CHECK_EQUAL(downloader.trimHostUrl("http://localhost:8080/some/path"), "http://localhost:8080");
+  BOOST_CHECK_EQUAL(downloader.trimHostUrl("http://localhost/some/path"), "http://localhost");
+  BOOST_CHECK_EQUAL(downloader.trimHostUrl("http://localhost:8080/Task/Detector/1?HTTPOnly=true"), "http://localhost:8080");
 }
 
 } // namespace ccdb