[kudu] branch master updated: [utils] Fix a data race when creating EasyCurl objects in multithreads

[laiyingchun]

This is an automated email from the ASF dual-hosted git repository.

laiyingchun pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git


The following commit(s) were added to refs/heads/master by this push:
     new 8258595  [utils] Fix a data race when creating EasyCurl objects in 
multithreads
8258595 is described below

commit 8258595030fea07906fce91eeff0b18e695402c2
Author: Yingchun Lai <405403...@qq.com>
AuthorDate: Sat Sep 7 00:36:43 2019 +0800

    [utils] Fix a data race when creating EasyCurl objects in multithreads
    
    When creating EasyCurl objects in a multi-threaded environment,
    non-thread safe function curl_global_init() may be called several
    times at the same time.
    Now we use std::call_once() to make sure it is called once.
    
    Change-Id: I651657487d9168e9865b6f1bfec394765ad67075
    Reviewed-on: http://gerrit.cloudera.org:8080/14189
    Tested-by: Kudu Jenkins
    Reviewed-by: Alexey Serbin <aser...@cloudera.com>
    Reviewed-by: Adar Dembo <a...@cloudera.com>
---
 src/kudu/util/curl_util-test.cc | 20 ++++++++++++++++++++
 src/kudu/util/curl_util.cc      |  9 ++++++++-
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/src/kudu/util/curl_util-test.cc b/src/kudu/util/curl_util-test.cc
index f20fb11..f43b6c7 100644
--- a/src/kudu/util/curl_util-test.cc
+++ b/src/kudu/util/curl_util-test.cc
@@ -19,10 +19,13 @@
 
 #include <gtest/gtest.h>
 
+#include "kudu/gutil/gscoped_ptr.h"
 #include "kudu/util/debug/sanitizer_scopes.h"
 #include "kudu/util/faststring.h"
 #include "kudu/util/monotime.h"
 #include "kudu/util/status.h"
+#include "kudu/util/test_macros.h"
+#include "kudu/util/threadpool.h"
 
 namespace kudu {
 
@@ -36,4 +39,21 @@ TEST(CurlUtilTest, TestTimeout) {
   ASSERT_TRUE(s.IsTimedOut());
 }
 
+TEST(CurlUtilTest, NonSharedObjectsBetweenThreads) {
+  const int kThreadCount = 8;
+  gscoped_ptr<ThreadPool> pool;
+  ThreadPoolBuilder("curl-util-test")
+      .set_min_threads(kThreadCount)
+      .set_max_threads(kThreadCount)
+      .Build(&pool);
+
+  for (int i = 0; i < kThreadCount; i++) {
+    ASSERT_OK(pool->SubmitFunc([&]() {
+      EasyCurl curl;
+    }));
+  }
+
+  pool->Shutdown();
+}
+
 } // namespace kudu
diff --git a/src/kudu/util/curl_util.cc b/src/kudu/util/curl_util.cc
index 21f02df..e80d481 100644
--- a/src/kudu/util/curl_util.cc
+++ b/src/kudu/util/curl_util.cc
@@ -19,6 +19,7 @@
 
 #include <cstddef>
 #include <cstdint>
+#include <mutex>
 #include <ostream>
 
 #include <curl/curl.h>
@@ -58,7 +59,13 @@ EasyCurl::EasyCurl() {
   // Use our own SSL initialization, and disable curl's.
   // Both of these calls are idempotent.
   security::InitializeOpenSSL();
-  CHECK_EQ(0, curl_global_init(CURL_GLOBAL_DEFAULT & ~CURL_GLOBAL_SSL));
+  // curl_global_init() is not thread safe and multiple calls have the
+  // same effect as one call.
+  // See more details: https://curl.haxx.se/libcurl/c/curl_global_init.html
+  static std::once_flag once;
+  std::call_once(once, []() {
+    CHECK_EQ(0, curl_global_init(CURL_GLOBAL_DEFAULT & ~CURL_GLOBAL_SSL));
+  });
   curl_ = curl_easy_init();
   CHECK(curl_) << "Could not init curl";
 }