From c00b6edeea93aabb1b5e6813c29aa7251af104b3 Mon Sep 17 00:00:00 2001
From: Kamil Cudnik <kcudnik@gmail.com>
Date: Tue, 3 May 2016 14:38:29 -0700
Subject: [PATCH] Fix race condition cold boot vs notifications (#14)

---
 common/redisclient.cpp        | 11 +++++++++--
 syncd/syncd.cpp               |  4 ++++
 syncd/syncd_notifications.cpp |  4 ++--
 syncd/syncd_reinit.cpp        | 21 ++++++++++-----------
 4 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/common/redisclient.cpp b/common/redisclient.cpp
index 78ccd737c4..0821d0549b 100644
--- a/common/redisclient.cpp
+++ b/common/redisclient.cpp
@@ -1,4 +1,5 @@
 #include "redisclient.h"
+#include "swss/logger.h"
 
 namespace swss
 {
@@ -190,7 +191,11 @@ std::shared_ptr<std::string> RedisClient::hget(std::string key, std::string fiel
     redisGetReply(m_db->getContext(), (void**)&reply);
 
     if (!reply)
-        throw std::runtime_error("HGET failed, memory exception");
+    {
+        SWSS_LOG_ERROR("HGET failed, null reply, memory exception: %s: %s", key.c_str(), field.c_str());
+
+        throw std::runtime_error("HGET failed, null reply, memory exception");
+    }
 
     if (reply->type == REDIS_REPLY_NIL)
     {
@@ -205,9 +210,11 @@ std::shared_ptr<std::string> RedisClient::hget(std::string key, std::string fiel
         return ptr;
     }
 
+    SWSS_LOG_ERROR("HGET failed, reply-type: %d, %s: %s", reply->type, key.c_str(), field.c_str());
+
     freeReplyObject(reply);
 
-    throw std::runtime_error("HGET failed, memory exception");
+    throw std::runtime_error("HGET failed, unexpected reply type, memory exception");
 }
 
 int64_t RedisClient::rpush(std::string list, std::string item)
diff --git a/syncd/syncd.cpp b/syncd/syncd.cpp
index 7231960fb6..6b3e665a9a 100644
--- a/syncd/syncd.cpp
+++ b/syncd/syncd.cpp
@@ -1212,4 +1212,8 @@ int main(int argc, char **argv)
     SWSS_LOG_NOTICE("calling api uninitialize");
 
     sai_api_uninitialize();
+
+    SWSS_LOG_NOTICE("uninitialize finished");
+
+    return EXIT_SUCCESS;
 }
diff --git a/syncd/syncd_notifications.cpp b/syncd/syncd_notifications.cpp
index 06ae5cef5f..8843543955 100644
--- a/syncd/syncd_notifications.cpp
+++ b/syncd/syncd_notifications.cpp
@@ -7,7 +7,7 @@ void send_notification(
 {
     SWSS_LOG_ENTER();
 
-    SWSS_LOG_DEBUG("sending notification: %s:%s", op.c_str(), data.c_str());
+    SWSS_LOG_NOTICE("%s %s", op.c_str(), data.c_str());
 
     notifications->send(op, data, entry);
 
@@ -39,7 +39,7 @@ void on_switch_state_change(
 }
 
 sai_fdb_entry_type_t getFdbEntryType(
-        _In_ uint32_t count, 
+        _In_ uint32_t count,
         _In_ const sai_attribute_t *list)
 {
     for (uint32_t idx = 0; idx < count; idx++)
diff --git a/syncd/syncd_reinit.cpp b/syncd/syncd_reinit.cpp
index f83e1fb4ba..17f990826c 100644
--- a/syncd/syncd_reinit.cpp
+++ b/syncd/syncd_reinit.cpp
@@ -472,17 +472,6 @@ void helperCheckPortIds()
 {
     SWSS_LOG_ENTER();
 
-    // we need lock here since after initialization
-    // some notifications may appear and it may cause
-    // race condition for port id generation
-    std::lock_guard<std::mutex> lock(g_mutex);
-
-    // it may happen that after initialize we will receive
-    // some port notifications with port'ids that are not in
-    // redis db yet, so after checking VIDTORID map there will
-    // be entries and translate_vid_to_rid will generate new
-    // id's for ports
-
     auto laneMap = saiGetHardwareLaneMap();
 
     for (auto kv: laneMap)
@@ -542,6 +531,16 @@ void helperCheckVlanId()
 
 void onSyncdStart(bool warmStart)
 {
+    // it may happen that after initialize we will receive
+    // some port notifications with port'ids that are not in
+    // redis db yet, so after checking VIDTORID map there will
+    // be entries and translate_vid_to_rid will generate new
+    // id's for ports, this may cause race condition so we need
+    // to use a lock here to prevent that
+
+    std::lock_guard<std::mutex> lock(g_mutex);
+
+
     SWSS_LOG_ENTER();
 
     helperCheckLaneMap();