MB-21034: Fix race in test_expiry_pager_settings (gmtime_r) 46/67946/5
authorDave Rigby <daver@couchbase.com>
Fri, 23 Sep 2016 13:59:34 +0000 (14:59 +0100)
committerDave Rigby <daver@couchbase.com>
Tue, 27 Sep 2016 18:22:58 +0000 (18:22 +0000)
When calculating expiry pager adjusted time values, use the
thread-safe variant (gmtime_r) so the test doesn't conflict with the
ep_engine code.

Also use std::chrono to perform the time manipulaton (which handles
any modulus of minutes -> hours etc).

Change-Id: Iee39a86b73a71642b9dab4ff2821d589699731ac
Reviewed-on: http://review.couchbase.org/67946
Reviewed-by: Manu Dhundi <manu@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
src/item_pager.cc
tests/ep_testsuite.cc

index 1bcf17d..b5d31a3 100644 (file)
@@ -321,7 +321,7 @@ ExpiredItemPager::ExpiredItemPager(EventuallyPersistentEngine *e,
          */
         time_t now = ep_abs_time(ep_current_time());
         struct tm timeNow, timeTarget;
-        timeNow = *(gmtime(&now));
+        cb_gmtime_r(&now, &timeNow);
         timeTarget = timeNow;
         if (timeNow.tm_hour >= (int)startTime) {
             timeTarget.tm_mday += 1;
index d5d3ff8..7714a86 100644 (file)
@@ -26,6 +26,7 @@
 
 #include <condition_variable>
 #include <cstdlib>
+#include <chrono>
 #include <iostream>
 #include <iomanip>
 #include <map>
@@ -725,6 +726,23 @@ static enum test_result test_wrong_vb_del(ENGINE_HANDLE *h, ENGINE_HANDLE_V1 *h1
     return SUCCESS;
 }
 
+/* Returns a string in the format "%Y-%m-%d %H:%M:%S" of the specified
+ * time point.
+ */
+std::string make_time_string(std::chrono::system_clock::time_point time_point) {
+    time_t tt = std::chrono::system_clock::to_time_t(time_point);
+#ifdef _MSC_VER
+    // Windows' gmtime() is already thread-safe.
+    struct tm* split = gmtime(&tt);
+#else
+    struct tm local_storage;
+    struct tm* split = gmtime_r(&tt, &local_storage);
+#endif
+    char timeStr[20];
+    strftime(timeStr, 20, "%Y-%m-%d %H:%M:%S", split);
+    return timeStr;
+}
+
 static enum test_result test_expiry_pager_settings(ENGINE_HANDLE *h,
                                                    ENGINE_HANDLE_V1 *h1) {
 
@@ -772,33 +790,16 @@ static enum test_result test_expiry_pager_settings(ENGINE_HANDLE *h,
     checkeq(0, str.substr(11, 5).compare(expected_time), err_msg.c_str());
 
     // Update exp_pager_stime by 30 minutes and ensure that the update is successful
-    int update_by = 30;
-    time_t now = time(NULL);
-    struct tm curr = *(gmtime(&now));
-    curr.tm_min += update_by;
-#ifdef _MSC_VER
-    _mkgmtime(&curr);
-#else
-    timegm(&curr);
-#endif
-    char timeStr[20];
-    strftime(timeStr, 20, "%Y-%m-%d %H:%M:%S", &curr);
-    std::string targetTaskTime1(timeStr);
+    const std::chrono::minutes update_by{30};
+    std::string targetTaskTime1{make_time_string(std::chrono::system_clock::now() +
+                                                 update_by)};
 
     set_param(h, h1, protocol_binary_engine_param_flush, "exp_pager_stime",
-              std::to_string(update_by * 60).c_str());
+              std::to_string(update_by.count() * 60).c_str());
     str = get_str_stat(h, h1, "ep_expiry_pager_task_time");
 
-    now = time(NULL);
-    curr = *(gmtime(&now));
-    curr.tm_min += update_by;
-#ifdef _MSC_VER
-    _mkgmtime(&curr);
-#else
-    timegm(&curr);
-#endif
-    strftime(timeStr, 20, "%Y-%m-%d %H:%M:%S", &curr);
-    std::string targetTaskTime2(timeStr);
+    std::string targetTaskTime2{make_time_string(std::chrono::system_clock::now() +
+                                                 update_by)};
 
     // ep_expiry_pager_task_time should fall within the range of
     // targetTaskTime1 and targetTaskTime2