From 564713144efe66e9c8c01d7e2516e3fc931ea032 Mon Sep 17 00:00:00 2001 From: Alessio Placitelli Date: Wed, 24 May 2017 19:27:33 +0200 Subject: [PATCH] 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 --HG-- extra : rebase_source : 4149973f832f968a75ec51b245104cb02fd35d4e --- testing/profiles/prefs_general.js | 3 +++ .../components/telemetry/pingsender/pingsender.cpp | 12 ++++++------ .../telemetry/pingsender/pingsender_unix_common.cpp | 10 +++++++++- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/testing/profiles/prefs_general.js b/testing/profiles/prefs_general.js index 1d92ef49377c..b4d49a82de2d 100644 --- a/testing/profiles/prefs_general.js +++ b/testing/profiles/prefs_general.js @@ -246,6 +246,9 @@ user_pref('browser.contentHandlers.types.5.uri', 'http://test1.example.org/rss?u // 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. diff --git a/toolkit/components/telemetry/pingsender/pingsender.cpp b/toolkit/components/telemetry/pingsender/pingsender.cpp index 014fada25be4..ffbed7fb36c2 100644 --- a/toolkit/components/telemetry/pingsender/pingsender.cpp +++ b/toolkit/components/telemetry/pingsender/pingsender.cpp @@ -152,14 +152,14 @@ int main(int argc, char* argv[]) "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. @@ -170,18 +170,18 @@ int main(int argc, char* argv[]) // 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; } diff --git a/toolkit/components/telemetry/pingsender/pingsender_unix_common.cpp b/toolkit/components/telemetry/pingsender/pingsender_unix_common.cpp index ca6936ad9b73..23159e862866 100644 --- a/toolkit/components/telemetry/pingsender/pingsender_unix_common.cpp +++ b/toolkit/components/telemetry/pingsender/pingsender_unix_common.cpp @@ -41,6 +41,7 @@ public: void (*slist_free_all)(curl_slist*); const char* (*easy_strerror)(CURLcode); void (*easy_cleanup)(CURL*); + void (*global_cleanup)(void); private: void* mLib; @@ -56,6 +57,7 @@ CurlWrapper::CurlWrapper() , slist_free_all(nullptr) , easy_strerror(nullptr) , easy_cleanup(nullptr) + , global_cleanup(nullptr) , mLib(nullptr) , mCurl(nullptr) {} @@ -67,6 +69,10 @@ CurlWrapper::~CurlWrapper() easy_cleanup(mCurl); } + if (global_cleanup) { + global_cleanup(); + } + dlclose(mLib); } } @@ -116,6 +122,7 @@ CurlWrapper::Init() *(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 || @@ -124,7 +131,8 @@ CurlWrapper::Init() !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; }