fix: time utils overflow, meta_split_service lock (#665)

apache · Nov 24, 2020 · f80fba8 · f80fba8
1 parent 5be3e47
commit f80fba8
Show file tree

Hide file tree

Showing 24 changed files with 80 additions and 42 deletions.
diff --git a/include/dsn/utility/time_utils.h → include/dsn/utils/time_utils.h b/include/dsn/utility/time_utils.h → include/dsn/utils/time_utils.h
@@ -27,7 +27,7 @@
 
 #include <chrono>
 #include <cstdio>
-#include "string_view.h"
+#include <dsn/utility/string_view.h>
 
 namespace dsn {
 namespace utils {
@@ -39,20 +39,7 @@ static struct tm *get_localtime(uint64_t ts_ms, struct tm *tm_buf)
 }
 
 // get time string, which format is yyyy-MM-dd hh:mm:ss.SSS
-inline void time_ms_to_string(uint64_t ts_ms, char *str)
-{
-    struct tm tmp;
-    auto ret = get_localtime(ts_ms, &tmp);
-    sprintf(str,
-            "%04d-%02d-%02d %02d:%02d:%02d.%03u",
-            ret->tm_year + 1900,
-            ret->tm_mon + 1,
-            ret->tm_mday,
-            ret->tm_hour,
-            ret->tm_min,
-            ret->tm_sec,
-            static_cast<uint32_t>(ts_ms % 1000));
-}
+extern void time_ms_to_string(uint64_t ts_ms, char *str);
 
 // get date string with format of 'yyyy-MM-dd' from given timestamp
 inline void time_ms_to_date(uint64_t ts_ms, char *str, int len)

diff --git a/src/client/replication_ddl_client.cpp b/src/client/replication_ddl_client.cpp
@@ -41,7 +41,7 @@
 #include <dsn/utility/error_code.h>
 #include <dsn/utility/output_utils.h>
 #include <fmt/format.h>
-#include <dsn/utility/time_utils.h>
+#include <dsn/utils/time_utils.h>
 
 #include "common/replication_common.h"
 

diff --git a/src/common/duplication_common.cpp b/src/common/duplication_common.cpp
@@ -28,7 +28,7 @@
 #include <dsn/dist/replication/duplication_common.h>
 #include <dsn/dist/fmt_logging.h>
 #include <dsn/utility/singleton.h>
-#include <dsn/utility/time_utils.h>
+#include <dsn/utils/time_utils.h>
 #include <nlohmann/json.hpp>
 
 namespace dsn {

diff --git a/src/http/builtin_http_calls.cpp b/src/http/builtin_http_calls.cpp
@@ -16,7 +16,7 @@
 // under the License.
 
 #include <dsn/utility/output_utils.h>
-#include <dsn/utility/time_utils.h>
+#include <dsn/utils/time_utils.h>
 
 #include "builtin_http_calls.h"
 #include "http_call_registry.h"

diff --git a/src/http/http_server.cpp b/src/http/http_server.cpp
@@ -4,7 +4,7 @@
 
 #include <dsn/http/http_server.h>
 #include <dsn/tool_api.h>
-#include <dsn/utility/time_utils.h>
+#include <dsn/utils/time_utils.h>
 #include <boost/algorithm/string.hpp>
 #include <fmt/ostream.h>
 

diff --git a/src/meta/meta_backup_service.cpp b/src/meta/meta_backup_service.cpp
@@ -16,7 +16,7 @@
 // under the License.
 
 #include <dsn/utility/filesystem.h>
-#include <dsn/utility/time_utils.h>
+#include <dsn/utils/time_utils.h>
 #include <dsn/utility/output_utils.h>
 #include <dsn/http/http_server.h>
 

diff --git a/src/meta/meta_http_service.cpp b/src/meta/meta_http_service.cpp
@@ -11,7 +11,7 @@
 #include <dsn/dist/replication/duplication_common.h>
 #include <dsn/utility/config_api.h>
 #include <dsn/utility/output_utils.h>
-#include <dsn/utility/time_utils.h>
+#include <dsn/utils/time_utils.h>
 
 #include "meta_http_service.h"
 #include "meta_server_failure_detector.h"

diff --git a/src/meta/meta_split_service.cpp b/src/meta/meta_split_service.cpp
@@ -135,7 +135,7 @@ void meta_split_service::register_child_on_meta(register_child_rpc rpc)
     auto &response = rpc.response();
     response.err = ERR_IO_PENDING;
 
-    zauto_write_lock(app_lock());
+    zauto_write_lock l(app_lock());
     std::shared_ptr<app_state> app = _state->get_app(app_name);
     dassert_f(app != nullptr, "app({}) is not existed", app_name);
     dassert_f(app->is_stateful, "app({}) is stateless currently", app_name);
@@ -242,7 +242,7 @@ void meta_split_service::on_add_child_on_remote_storage_reply(error_code ec,
     auto &response = rpc.response();
     const std::string &app_name = request.app.app_name;
 
-    zauto_write_lock(app_lock());
+    zauto_write_lock l(app_lock());
     std::shared_ptr<app_state> app = _state->get_app(app_name);
     dassert_f(app != nullptr, "app({}) is not existed", app_name);
     dassert_f(app->is_stateful, "app({}) is stateless currently", app_name);

diff --git a/src/meta/server_load_balancer.cpp b/src/meta/server_load_balancer.cpp
@@ -3,7 +3,7 @@
 #include <dsn/utility/string_conv.h>
 #include <dsn/tool-api/command_manager.h>
 #include <boost/lexical_cast.hpp>
-#include <dsn/utility/time_utils.h>
+#include <dsn/utils/time_utils.h>
 
 namespace dsn {
 namespace replication {

diff --git a/src/meta/test/backup_test.cpp b/src/meta/test/backup_test.cpp
@@ -1,6 +1,6 @@
 #include <gtest/gtest.h>
 #include <dsn/service_api_cpp.h>
-#include <dsn/utility/time_utils.h>
+#include <dsn/utils/time_utils.h>
 
 #include "meta/meta_backup_service.h"
 #include "meta/meta_service.h"

diff --git a/src/meta/test/meta_duplication_service_test.cpp b/src/meta/test/meta_duplication_service_test.cpp
@@ -26,7 +26,7 @@
 
 #include <gtest/gtest.h>
 #include <dsn/dist/fmt_logging.h>
-#include <dsn/utility/time_utils.h>
+#include <dsn/utils/time_utils.h>
 
 #include "meta/server_load_balancer.h"
 #include "meta/meta_server_failure_detector.h"

diff --git a/src/perf_counter/perf_counter_atomic.h b/src/perf_counter/perf_counter_atomic.h
@@ -8,7 +8,7 @@
 #include <dsn/utility/config_api.h>
 #include <dsn/c/api_utilities.h>
 #include <dsn/perf_counter/perf_counter.h>
-#include <dsn/utility/time_utils.h>
+#include <dsn/utils/time_utils.h>
 #include "utils/shared_io_service.h"
 
 namespace dsn {

diff --git a/src/perf_counter/perf_counters.cpp b/src/perf_counter/perf_counters.cpp
@@ -36,7 +36,7 @@
 #include <dsn/tool-api/command_manager.h>
 #include <dsn/tool-api/task.h>
 #include <dsn/utility/string_view.h>
-#include <dsn/utility/time_utils.h>
+#include <dsn/utils/time_utils.h>
 
 #include "perf_counter_atomic.h"
 #include "builtin_counters.h"

diff --git a/src/replica/replica_backup.cpp b/src/replica/replica_backup.cpp
@@ -1,7 +1,7 @@
 #include <boost/lexical_cast.hpp>
 
 #include <dsn/utility/filesystem.h>
-#include <dsn/utility/time_utils.h>
+#include <dsn/utils/time_utils.h>
 #include <dsn/dist/fmt_logging.h>
 #include <dsn/dist/replication/replication_app_base.h>
 #include <dsn/utility/flags.h>

diff --git a/src/runtime/profiler_command.cpp b/src/runtime/profiler_command.cpp
@@ -42,7 +42,7 @@
 #include <dsn/toollet/profiler.h>
 #include "profiler_header.h"
 #include <dsn/cpp/json_helper.h>
-#include <dsn/utility/time_utils.h>
+#include <dsn/utils/time_utils.h>
 
 namespace dsn {
 namespace tools {

diff --git a/src/runtime/rpc/rpc_message.cpp b/src/runtime/rpc/rpc_message.cpp
@@ -181,7 +181,7 @@ message_ex *message_ex::copy_message_no_reply(const message_ex &old_msg)
         static_cast<char *>(dsn::tls_trans_malloc(sizeof(message_header))),
         [](char *c) { dsn::tls_trans_free(c); });
     msg->header = reinterpret_cast<message_header *>(header_holder.get());
-    memset(msg->header, 0, sizeof(message_header));
+    msg->header = {}; // initialize to empty struct
     msg->buffers.emplace_back(blob(std::move(header_holder), sizeof(message_header)));
 
     if (old_msg.buffers.size() == 1) {

diff --git a/src/runtime/security/kinit_context.cpp b/src/runtime/security/kinit_context.cpp
@@ -23,7 +23,7 @@
 #include <krb5/krb5.h>
 
 #include <dsn/utility/defer.h>
-#include <dsn/utility/time_utils.h>
+#include <dsn/utils/time_utils.h>
 #include <dsn/dist/fmt_logging.h>
 #include <dsn/utility/flags.h>
 #include <dsn/utility/filesystem.h>

diff --git a/src/runtime/service_api_c.cpp b/src/runtime/service_api_c.cpp
@@ -39,7 +39,7 @@
 #include <dsn/utility/filesystem.h>
 #include <dsn/utility/process_utils.h>
 #include <dsn/utility/flags.h>
-#include <dsn/utility/time_utils.h>
+#include <dsn/utils/time_utils.h>
 #include <dsn/utility/errors.h>
 #include <dsn/dist/fmt_logging.h>
 

diff --git a/src/tools/mutation_log_tool.cpp b/src/tools/mutation_log_tool.cpp
@@ -25,7 +25,7 @@
  */
 
 #include <dsn/dist/replication/mutation_log_tool.h>
-#include <dsn/utility/time_utils.h>
+#include <dsn/utils/time_utils.h>
 #include "replica/mutation_log.h"
 
 namespace dsn {

diff --git a/src/utils/clock.cpp b/src/utils/clock.cpp
@@ -3,7 +3,7 @@
 // can be found in the LICENSE file in the root directory of this source tree.
 
 #include <dsn/utility/clock.h>
-#include <dsn/utility/time_utils.h>
+#include <dsn/utils/time_utils.h>
 #include <dsn/utility/dlib.h>
 #include <dsn/utility/smart_pointers.h>
 

diff --git a/src/utils/process_utils.cpp b/src/utils/process_utils.cpp
@@ -9,7 +9,7 @@
 #include <dsn/utility/utils.h>
 #include <dsn/utility/preloadable.h>
 #include <dsn/utility/process_utils.h>
-#include <dsn/utility/time_utils.h>
+#include <dsn/utils/time_utils.h>
 
 namespace dsn {
 namespace utils {

diff --git a/src/utils/simple_logger.cpp b/src/utils/simple_logger.cpp
@@ -28,7 +28,7 @@
 #include <sstream>
 #include <dsn/utility/filesystem.h>
 #include <dsn/utility/flags.h>
-#include <dsn/utility/time_utils.h>
+#include <dsn/utils/time_utils.h>
 
 namespace dsn {
 namespace tools {

diff --git a/src/utils/test/time_utils_test.cpp b/src/utils/test/time_utils_test.cpp
@@ -24,13 +24,14 @@
  * THE SOFTWARE.
  */
 
-#include <dsn/utility/time_utils.h>
+#include <dsn/utils/time_utils.h>
 #include <gtest/gtest.h>
+#include <dsn/c/api_layer1.h>
 
 namespace dsn {
 namespace utils {
 
-TEST(core, hh_mm_to_seconds)
+TEST(time_utils, hh_mm_to_seconds)
 {
     ASSERT_EQ(hh_mm_to_seconds("00:00"), 0);
     ASSERT_EQ(hh_mm_to_seconds("23:59"), 86340);
@@ -51,15 +52,15 @@ TEST(core, hh_mm_to_seconds)
     ASSERT_EQ(hh_mm_to_seconds("01b"), -1);
 }
 
-TEST(core, get_unix_sec_today_midnight)
+TEST(time_utils, get_unix_sec_today_midnight)
 {
     ASSERT_LT(0, get_unix_sec_today_midnight());
     ASSERT_LE(get_unix_sec_today_midnight(), time(nullptr));
     ASSERT_GE(time(nullptr) - get_unix_sec_today_midnight(), 0);
     ASSERT_LT(time(nullptr) - get_unix_sec_today_midnight(), 86400);
 }
 
-TEST(core, hh_mm_today_to_unix_sec)
+TEST(time_utils, hh_mm_today_to_unix_sec)
 {
     ASSERT_EQ(get_unix_sec_today_midnight() + hh_mm_to_seconds("0:0"),
               hh_mm_today_to_unix_sec("0:0"));
@@ -78,13 +79,28 @@ TEST(core, hh_mm_today_to_unix_sec)
     ASSERT_EQ(hh_mm_today_to_unix_sec("01b"), -1);
 }
 
-TEST(core, get_current_physical_time_ns)
+TEST(time_utils, get_current_physical_time_ns)
 {
     int64_t ts_ns = get_current_physical_time_ns();
     ASSERT_LT(0, ts_ns);
     ASSERT_GE(get_current_physical_time_ns() - ts_ns, 0);
     ASSERT_LT(get_current_physical_time_ns() - ts_ns, 1e7); // < 10 ms
 }
 
+TEST(time_utils, time_ms_to_string)
+{
+    char buf[64];
+    time_ms_to_string(1605091506136, buf);
+    // time differ between time zones,
+    // the real time 2020-11-11 18:45:06.136 (UTC+8)
+    // so it must be 2020-11-1x xx:45:06.136
+    ASSERT_EQ(std::string(buf).substr(0, 9), "2020-11-1");
+    ASSERT_EQ(std::string(buf).substr(13, 10), ":45:06.136");
+
+    memset(buf, 0, sizeof(buf));
+    time_ms_to_string(dsn_now_ms(), buf);
+    ASSERT_EQ(strlen(buf), strlen("XXXX-XX-XX XX:XX:XX.XXX"));
+}
+
 } // namespace utils
 } // namespace dsn
diff --git a/src/utils/time_utils.cpp b/src/utils/time_utils.cpp
@@ -0,0 +1,35 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <dsn/utils/time_utils.h>
+#include <fmt/chrono.h>
+#include <fmt/printf.h>
+
+namespace dsn {
+namespace utils {
+
+/*extern*/ void time_ms_to_string(uint64_t ts_ms, char *str)
+{
+    struct tm tmp;
+    auto ret = get_localtime(ts_ms, &tmp);
+    fmt::format_to(str, "{:%Y-%m-%d %H:%M:%S}.{}", *ret, static_cast<uint32_t>(ts_ms % 1000));
+}
+
+} // namespace utils
+} // namespace dsn