Bug 1364068 - Properly cleanup libcurl in the pingsender. r=gsvelto,jseward
authorAlessio Placitelli <alessio.placitelli@gmail.com>
Wed, 24 May 2017 19:27:33 +0200
changeset 409211 e61ba4616b8116937aad46222fee0670a0a409ef
parent 409210 cc274092e3782574f2ca8460ff5b6342813d05ef
child 409212 024c52408261d759497f856579112b1a779729f7
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgsvelto, jseward
bugs1364068
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1364068 - Properly cleanup libcurl in the pingsender. r=gsvelto,jseward This additionally changes exit() calls with |return VALUE| so that we are sure to call the destructors and valgrind doesn't complain. Moreover, this disables the 'new-profile' ping when Firefox is ran on test profiles. MozReview-Commit-ID: BlGE9w6yGCL
testing/profiles/prefs_general.js
toolkit/components/telemetry/pingsender/pingsender.cpp
toolkit/components/telemetry/pingsender/pingsender_unix_common.cpp
--- a/testing/profiles/prefs_general.js
+++ b/testing/profiles/prefs_general.js
@@ -241,16 +241,19 @@ user_pref('browser.contentHandlers.types
 user_pref('browser.contentHandlers.types.1.uri', 'http://test1.example.org/rss?url=%%s')
 user_pref('browser.contentHandlers.types.2.uri', 'http://test1.example.org/rss?url=%%s')
 user_pref('browser.contentHandlers.types.3.uri', 'http://test1.example.org/rss?url=%%s')
 user_pref('browser.contentHandlers.types.4.uri', 'http://test1.example.org/rss?url=%%s')
 user_pref('browser.contentHandlers.types.5.uri', 'http://test1.example.org/rss?url=%%s')
 
 // We want to collect telemetry, but we don't want to send in the results.
 user_pref('toolkit.telemetry.server', 'https://%(server)s/telemetry-dummy/');
+// Don't new-profile' ping on new profiles during tests, otherwise the testing framework
+// might wait on the pingsender to finish and slow down tests.
+user_pref("toolkit.telemetry.newProfilePing.enabled", false);
 
 // A couple of preferences with default values to test that telemetry preference
 // watching is working.
 user_pref('toolkit.telemetry.test.pref1', true);
 user_pref('toolkit.telemetry.test.pref2', false);
 
 // We don't want to hit the real Firefox Accounts server for tests.  We don't
 // actually need a functioning FxA server, so just set it to something that
--- a/toolkit/components/telemetry/pingsender/pingsender.cpp
+++ b/toolkit/components/telemetry/pingsender/pingsender.cpp
@@ -147,41 +147,41 @@ int main(int argc, char* argv[])
   if (argc == 3) {
     url = argv[1];
     pingPath = argv[2];
   } else {
     PINGSENDER_LOG("Usage: pingsender URL PATH\n"
                    "Send the payload stored in PATH to the specified URL using "
                    "an HTTP POST message\n"
                    "then delete the file after a successful send.\n");
-    exit(EXIT_FAILURE);
+    return EXIT_FAILURE;
   }
 
   string ping(ReadPing(pingPath));
 
   if (ping.empty()) {
     PINGSENDER_LOG("ERROR: Ping payload is empty\n");
-    exit(EXIT_FAILURE);
+    return EXIT_FAILURE;
   }
 
   // Compress the ping using gzip.
   string gzipPing(GzipCompress(ping));
 
   // In the unlikely event of failure to gzip-compress the ping, don't
   // attempt to send it uncompressed: Telemetry will pick it up and send
   // it compressed.
   if (gzipPing.empty()) {
     PINGSENDER_LOG("ERROR: Ping compression failed\n");
-    exit(EXIT_FAILURE);
+    return EXIT_FAILURE;
   }
 
   if (!Post(url, gzipPing)) {
-    exit(EXIT_FAILURE);
+    return EXIT_FAILURE;
   }
 
   // If the ping was successfully sent, delete the file.
   if (!pingPath.empty() && std::remove(pingPath.c_str())) {
     // We failed to remove the pending ping file.
-    exit(EXIT_FAILURE);
+    return EXIT_FAILURE;
   }
 
-  exit(EXIT_SUCCESS);
+  return EXIT_SUCCESS;
 }
--- a/toolkit/components/telemetry/pingsender/pingsender_unix_common.cpp
+++ b/toolkit/components/telemetry/pingsender/pingsender_unix_common.cpp
@@ -36,42 +36,48 @@ public:
   CURL* (*easy_init)(void);
   CURLcode (*easy_setopt)(CURL*, CURLoption, ...);
   CURLcode (*easy_perform)(CURL*);
   CURLcode (*easy_getinfo)(CURL*, CURLINFO, ...);
   curl_slist* (*slist_append)(curl_slist*, const char*);
   void (*slist_free_all)(curl_slist*);
   const char* (*easy_strerror)(CURLcode);
   void (*easy_cleanup)(CURL*);
+  void (*global_cleanup)(void);
 
 private:
   void* mLib;
   void* mCurl;
 };
 
 CurlWrapper::CurlWrapper()
   : easy_init(nullptr)
   , easy_setopt(nullptr)
   , easy_perform(nullptr)
   , easy_getinfo(nullptr)
   , slist_append(nullptr)
   , slist_free_all(nullptr)
   , easy_strerror(nullptr)
   , easy_cleanup(nullptr)
+  , global_cleanup(nullptr)
   , mLib(nullptr)
   , mCurl(nullptr)
 {}
 
 CurlWrapper::~CurlWrapper()
 {
   if(mLib) {
     if(mCurl && easy_cleanup) {
       easy_cleanup(mCurl);
     }
 
+    if (global_cleanup) {
+      global_cleanup();
+    }
+
     dlclose(mLib);
   }
 }
 
 bool
 CurlWrapper::Init()
 {
   // libcurl might show up under different names, try them all until we find it
@@ -111,25 +117,27 @@ CurlWrapper::Init()
   *(void**) (&easy_init) = dlsym(mLib, "curl_easy_init");
   *(void**) (&easy_setopt) = dlsym(mLib, "curl_easy_setopt");
   *(void**) (&easy_perform) = dlsym(mLib, "curl_easy_perform");
   *(void**) (&easy_getinfo) = dlsym(mLib, "curl_easy_getinfo");
   *(void**) (&slist_append) = dlsym(mLib, "curl_slist_append");
   *(void**) (&slist_free_all) = dlsym(mLib, "curl_slist_free_all");
   *(void**) (&easy_strerror) = dlsym(mLib, "curl_easy_strerror");
   *(void**) (&easy_cleanup) = dlsym(mLib, "curl_easy_cleanup");
+  *(void**) (&global_cleanup) = dlsym(mLib, "curl_global_cleanup");
 
   if (!easy_init ||
       !easy_setopt ||
       !easy_perform ||
       !easy_getinfo ||
       !slist_append ||
       !slist_free_all ||
       !easy_strerror ||
-      !easy_cleanup) {
+      !easy_cleanup ||
+      !global_cleanup) {
     PINGSENDER_LOG("ERROR: libcurl is missing one of the required symbols\n");
     return false;
   }
 
   mCurl = easy_init();
 
   if (!mCurl) {
     PINGSENDER_LOG("ERROR: Could not initialize libcurl\n");