Fix data race for accessing database in some commands (#253) (#258)

Before this commit, in spop, zremrangebylex, zremrangebyscore commands, the lock guard for accessing the database may not work actually.

Other changes
- Use 'localtime_r' instead of 'localtime'
- Use 'std::this_thread::get_id()' to get thread id

(cherry picked from commit 1ac04ca)

Co-authored-by: Calvin Xiao <calvin325@gmail.com>

src/redis_set.cc

@@ -2,6 +2,7 @@
 
 #include <map>
 #include <iostream>
+#include <memory>
 
 namespace Redis {
 
@@ -168,7 +169,9 @@ rocksdb::Status Set::Take(const Slice &user_key, std::vector<std::string> *membe
   std::string ns_key;
   AppendNamespacePrefix(user_key, &ns_key);
 
-  if (pop) LockGuard guard(storage_->GetLockManager(), ns_key);
+  std::unique_ptr<LockGuard> lock_guard;
+  if (pop) lock_guard = std::unique_ptr<LockGuard>(new LockGuard(storage_->GetLockManager(), ns_key));
+
   SetMetadata metadata(false);
   rocksdb::Status s = GetMetadata(ns_key, &metadata);
   if (!s.ok()) return s.IsNotFound() ? rocksdb::Status::OK() : s;

src/redis_zset.cc

@@ -4,6 +4,7 @@
 #include <map>
 #include <limits>
 #include <cmath>
+#include <memory>
 
 namespace Redis {
 
@@ -168,7 +169,9 @@ rocksdb::Status ZSet::Range(const Slice &user_key, int start, int stop, uint8_t
 
   bool removed = (flags & (uint8_t)ZSET_REMOVED) != 0;
   bool reversed = (flags & (uint8_t)ZSET_REVERSED) != 0;
-  if (removed) LockGuard guard(storage_->GetLockManager(), ns_key);
+
+  std::unique_ptr<LockGuard> lock_guard;
+  if (removed) lock_guard = std::unique_ptr<LockGuard>(new LockGuard(storage_->GetLockManager(), ns_key));
   ZSetMetadata metadata(false);
   rocksdb::Status s = GetMetadata(ns_key, &metadata);
   if (!s.ok()) return s.IsNotFound()? rocksdb::Status::OK():s;
@@ -239,7 +242,8 @@ rocksdb::Status ZSet::RangeByScore(const Slice &user_key,
   std::string ns_key;
   AppendNamespacePrefix(user_key, &ns_key);
 
-  if (spec.removed) LockGuard guard(storage_->GetLockManager(), ns_key);
+  std::unique_ptr<LockGuard> lock_guard;
+  if (spec.removed) lock_guard = std::unique_ptr<LockGuard>(new LockGuard(storage_->GetLockManager(), ns_key));
   ZSetMetadata metadata(false);
   rocksdb::Status s = GetMetadata(ns_key, &metadata);
   if (!s.ok()) return s.IsNotFound()? rocksdb::Status::OK():s;

src/server.cc

@@ -83,7 +83,8 @@ Status Server::Start() {
       if (is_loading_ == false && ++counter % 600 == 0  // check every minute
           && config_->compaction_checker_range.Enabled()) {
         auto now = std::time(nullptr);
-        auto local_time = std::localtime(&now);
+        std::tm *local_time;
+        localtime_r(&now, local_time);
         if (local_time->tm_hour >= config_->compaction_checker_range.Start
         && local_time->tm_hour <= config_->compaction_checker_range.Stop) {
           std::vector<std::string> cf_names = {Engine::kMetadataColumnFamilyName,
@@ -484,7 +485,8 @@ void Server::cron() {
     // check every 20s (use 20s instead of 60s so that cron will execute in critical condition)
     if (is_loading_ == false && counter != 0 && counter % 200 == 0) {
       auto t = std::time(nullptr);
-      auto now = std::localtime(&t);
+      std::tm *now;
+      localtime_r(&t, now);
       // disable compaction cron when the compaction checker was enabled
       if (!config_->compaction_checker_range.Enabled()
           && config_->compact_cron.IsEnabled()

src/worker.cc

@@ -365,7 +365,7 @@ void WorkerThread::Start() {
       } else {
         Util::ThreadSetName("worker");
       }
-      this->worker_->Run(t_.get_id());
+      this->worker_->Run(std::this_thread::get_id());
     });
   } catch (const std::system_error &e) {
     LOG(ERROR) << "[worker] Failed to start worker thread, err: " << e.what();