From a4b8a5a28701d0a6444d5fae94835760a1d2358e Mon Sep 17 00:00:00 2001
From: Hua Liu <58683130+liuh-80@users.noreply.github.com>
Date: Mon, 1 Aug 2022 13:25:06 +0800
Subject: [PATCH] Fix memory leak issue in ConfigDBConnector. (#655)

Fix memory leak issue in ConfigDBConnector:
[chassis] Too many open files error and unable to connect to redis socket error
https://github.com/Azure/sonic-buildimage/issues/10870

The reason of this issue is DBConnector::pubsub() will return a pointer, and following code call this method but never release the returned pointer:

```
void ConfigDBConnector_Native::db_connect(string db_name, bool wait_for_init, bool retry_on)
{
    m_db_name = db_name;
    m_key_separator = m_table_name_separator = get_db_separator(db_name);
    SonicV2Connector_Native::connect(m_db_name, retry_on);

    if (wait_for_init)
    {
        auto& client = get_redis_client(m_db_name);
        auto pubsub = client.pubsub(); <== this pointer not delete later.
```

Also change DBConnector::pubsub() to deprecated for none SWIG scenario.

Change DBConnector::pubsub() to return a smart pointer.

Pass all test case.

Run following code in python and validate there is no epoll and socket leak:
```
import gc
from swsscommon import swsscommon
config_db = swsscommon.ConfigDBConnector_Native()
config_db.connect()
config_db.connect()
config_db.connect()

gc.collect()
```

<!--
- Note we only backport fixes to a release branch, *not* features!
- Please also provide a reason for the backporting below.
- e.g.
- [x] 202006
-->

- [ ] 201811
- [ ] 201911
- [ ] 202006
- [ ] 202012
- [ ] 202106
- [x] 202111

Fix epoll and socket resurce leak issue:
[chassis] Too many open files error and unable to connect to redis socket error
https://github.com/Azure/sonic-buildimage/issues/10870

Co-authored-by: liuh-80 <azureuser@liuh-dev-vm-02.5fg3zjdzj2xezlx1yazx5oxkzd.hx.internal.cloudapp.net>
---
 common/configdb.cpp    |  2 +-
 common/dbconnector.h   |  3 +++
 tests/test_redis_ut.py | 15 +++++++++++++++
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/common/configdb.cpp b/common/configdb.cpp
index 6d8747bf3..c6939b915 100644
--- a/common/configdb.cpp
+++ b/common/configdb.cpp
@@ -24,7 +24,7 @@ void ConfigDBConnector_Native::db_connect(string db_name, bool wait_for_init, bo
     if (wait_for_init)
     {
         auto& client = get_redis_client(m_db_name);
-        auto pubsub = client.pubsub();
+        auto pubsub = make_shared<PubSub>(&client);
         auto initialized = client.get(INIT_INDICATOR);
         if (!initialized || initialized->empty())
         {
diff --git a/common/dbconnector.h b/common/dbconnector.h
index bdd410f55..5ec2877c1 100644
--- a/common/dbconnector.h
+++ b/common/dbconnector.h
@@ -172,6 +172,9 @@ class DBConnector : public RedisContext
     /* Create new context to DB */
     DBConnector *newConnector(unsigned int timeout) const;
 
+#ifndef SWIG
+    __attribute__((deprecated))
+#endif
     PubSub *pubsub();
 
     int64_t del(const std::string &key);
diff --git a/tests/test_redis_ut.py b/tests/test_redis_ut.py
index 58b0b48e6..0fc54acfe 100644
--- a/tests/test_redis_ut.py
+++ b/tests/test_redis_ut.py
@@ -381,6 +381,7 @@ def test_ConfigDBConnector():
 def test_ConfigDBConnectorSeparator():
     db = swsscommon.DBConnector("APPL_DB", 0, True)
     config_db = ConfigDBConnector()
+    # set wait for init to True to cover wait_for_init code.
     config_db.db_connect("APPL_DB", False, False)
     config_db.get_redis_client(config_db.APPL_DB).flushdb()
     config_db.set_entry("TEST_PORT", "Ethernet222", {"alias": "etp2x"})
@@ -677,3 +678,17 @@ def test_DBConnectFailure():
     with pytest.raises(RuntimeError):
         db = swsscommon.DBConnector(0, nonexisting_host, 0)
 
+def test_ConfigDBWaitInit():
+    config_db = ConfigDBConnector()
+    config_db.connect(wait_for_init=False)
+    client = config_db.get_redis_client(config_db.CONFIG_DB)
+    suc = client.set(config_db.INIT_INDICATOR, 1)
+    assert suc
+
+    # set wait for init to True to cover wait_for_init code.
+    config_db = ConfigDBConnector()
+    config_db.db_connect(config_db.CONFIG_DB, True, False)
+
+    config_db.set_entry("TEST_PORT", "Ethernet111", {"alias": "etp1x"})
+    allconfig = config_db.get_config()
+    assert allconfig["TEST_PORT"]["Ethernet111"]["alias"] == "etp1x"