From 7759b78eb2054ce4945b561f082c112fb9c0a12d Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Wed, 27 Jul 2022 06:53:56 +0000 Subject: [PATCH 1/6] Fix memory leak issue. --- common/dbconnector.cpp | 4 ++-- common/dbconnector.h | 2 +- pyext/swsscommon.i | 1 - 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/common/dbconnector.cpp b/common/dbconnector.cpp index ac17928a5..3cb59045f 100755 --- a/common/dbconnector.cpp +++ b/common/dbconnector.cpp @@ -628,9 +628,9 @@ DBConnector *DBConnector::newConnector(unsigned int timeout) const return ret; } -PubSub *DBConnector::pubsub() +shared_ptr DBConnector::pubsub() { - return new PubSub(this); + return make_shared(this); } int64_t DBConnector::del(const string &key) diff --git a/common/dbconnector.h b/common/dbconnector.h index 88e6153fe..1f20a591e 100644 --- a/common/dbconnector.h +++ b/common/dbconnector.h @@ -174,7 +174,7 @@ class DBConnector : public RedisContext /* Create new context to DB */ DBConnector *newConnector(unsigned int timeout) const; - PubSub *pubsub(); + std::shared_ptr pubsub(); int64_t del(const std::string &key); diff --git a/pyext/swsscommon.i b/pyext/swsscommon.i index cbbf8a4e2..b2b025676 100644 --- a/pyext/swsscommon.i +++ b/pyext/swsscommon.i @@ -145,7 +145,6 @@ T castSelectableObj(swss::Selectable *temp) // Handle object ownership issue with %newobject: // https://www.swig.org/Doc4.0/SWIGDocumentation.html#Customization_ownership // %newobject must declared before %include header files -%newobject swss::DBConnector::pubsub; %newobject swss::DBConnector::newConnector; %include "schema.h" From 14ca06da3aed691c9e9df9543f4e5dd59e41111a Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Wed, 27 Jul 2022 08:29:17 +0000 Subject: [PATCH 2/6] Fix return object issue --- pyext/swsscommon.i | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pyext/swsscommon.i b/pyext/swsscommon.i index b2b025676..96daa22c5 100644 --- a/pyext/swsscommon.i +++ b/pyext/swsscommon.i @@ -147,6 +147,10 @@ T castSelectableObj(swss::Selectable *temp) // %newobject must declared before %include header files %newobject swss::DBConnector::newConnector; +// Return shared_ptr from DBConnector::pubsub(): +// https://www.swig.org/Doc4.0/Library.html#Library_shared_ptr_basics +%shared_ptr(PubSub); + %include "schema.h" %include "dbconnector.h" %include "sonicv2connector.h" From debe50841286486ded2192f8d2ca4cd1db3a26c3 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Wed, 27 Jul 2022 08:47:01 +0000 Subject: [PATCH 3/6] Fix name space issue --- pyext/swsscommon.i | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyext/swsscommon.i b/pyext/swsscommon.i index 96daa22c5..f592d9344 100644 --- a/pyext/swsscommon.i +++ b/pyext/swsscommon.i @@ -149,7 +149,7 @@ T castSelectableObj(swss::Selectable *temp) // Return shared_ptr from DBConnector::pubsub(): // https://www.swig.org/Doc4.0/Library.html#Library_shared_ptr_basics -%shared_ptr(PubSub); +%shared_ptr(swss::PubSub) %include "schema.h" %include "dbconnector.h" From f211ecb1497ab7139297a35dc587dfb07ceb6ab6 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Thu, 28 Jul 2022 04:44:46 +0000 Subject: [PATCH 4/6] Improve code by PR comments --- common/configdb.cpp | 2 +- common/dbconnector.cpp | 4 ++-- common/dbconnector.h | 5 ++++- pyext/swsscommon.i | 5 +---- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/common/configdb.cpp b/common/configdb.cpp index d1aab140f..2d733f9f6 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(&client); auto initialized = client.get(INIT_INDICATOR); if (!initialized || initialized->empty()) { diff --git a/common/dbconnector.cpp b/common/dbconnector.cpp index 3cb59045f..ac17928a5 100755 --- a/common/dbconnector.cpp +++ b/common/dbconnector.cpp @@ -628,9 +628,9 @@ DBConnector *DBConnector::newConnector(unsigned int timeout) const return ret; } -shared_ptr DBConnector::pubsub() +PubSub *DBConnector::pubsub() { - return make_shared(this); + return new PubSub(this); } int64_t DBConnector::del(const string &key) diff --git a/common/dbconnector.h b/common/dbconnector.h index 1f20a591e..eb37a6e83 100644 --- a/common/dbconnector.h +++ b/common/dbconnector.h @@ -174,7 +174,10 @@ class DBConnector : public RedisContext /* Create new context to DB */ DBConnector *newConnector(unsigned int timeout) const; - std::shared_ptr pubsub(); +#ifndef SWIG + __attribute__((deprecated)) +#endif + PubSub *pubsub(); int64_t del(const std::string &key); diff --git a/pyext/swsscommon.i b/pyext/swsscommon.i index f592d9344..cbbf8a4e2 100644 --- a/pyext/swsscommon.i +++ b/pyext/swsscommon.i @@ -145,12 +145,9 @@ T castSelectableObj(swss::Selectable *temp) // Handle object ownership issue with %newobject: // https://www.swig.org/Doc4.0/SWIGDocumentation.html#Customization_ownership // %newobject must declared before %include header files +%newobject swss::DBConnector::pubsub; %newobject swss::DBConnector::newConnector; -// Return shared_ptr from DBConnector::pubsub(): -// https://www.swig.org/Doc4.0/Library.html#Library_shared_ptr_basics -%shared_ptr(swss::PubSub) - %include "schema.h" %include "dbconnector.h" %include "sonicv2connector.h" From 85da7fd4fba4969f98862b898f3451c37859483e Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Thu, 28 Jul 2022 08:40:10 +0000 Subject: [PATCH 5/6] Improve code coverage --- tests/test_redis_ut.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_redis_ut.py b/tests/test_redis_ut.py index 344a87ef1..9e32424ce 100644 --- a/tests/test_redis_ut.py +++ b/tests/test_redis_ut.py @@ -381,7 +381,8 @@ def test_ConfigDBConnector(): def test_ConfigDBConnectorSeparator(): db = swsscommon.DBConnector("APPL_DB", 0, True) config_db = ConfigDBConnector() - config_db.db_connect("APPL_DB", False, False) + # set wait for init to True to cover wait_for_init code. + config_db.db_connect("APPL_DB", True, False) config_db.get_redis_client(config_db.APPL_DB).flushdb() config_db.set_entry("TEST_PORT", "Ethernet222", {"alias": "etp2x"}) db.set("ItemWithoutSeparator", "item11") From 2600d2c95d4eb96a0a47c4c700dca55d3bb650b3 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Fri, 29 Jul 2022 02:28:34 +0000 Subject: [PATCH 6/6] Add new UT to improve code coverage --- tests/test_redis_ut.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/tests/test_redis_ut.py b/tests/test_redis_ut.py index 9e32424ce..74c2510cb 100644 --- a/tests/test_redis_ut.py +++ b/tests/test_redis_ut.py @@ -382,7 +382,7 @@ 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", True, False) + 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"}) db.set("ItemWithoutSeparator", "item11") @@ -697,3 +697,18 @@ def test_SonicV2Connector(): db.set("TEST_DB", "test_key", "field1", 1) value = db.get("TEST_DB", "test_key", "field1") assert value == "1" + +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"