nvs: fix memory leaks in HashList and nvs_close

Fixes TW8162.
Associated test case is run under Instruments on macOS, until I set up valgrind to test this automatically on Linux.

diff --git a/components/nvs_flash/src/nvs_api.cpp b/components/nvs_flash/src/nvs_api.cpp
index 0a56925c0c863cdab6609ce3cfff08f356a2afdf..c1a910260e1d3a73f0012a5385d69011878b4aac 100644 (file)
--- a/components/nvs_flash/src/nvs_api.cpp
+++ b/components/nvs_flash/src/nvs_api.cpp
@@ -116,6 +116,7 @@ extern "C" void nvs_close(nvs_handle handle)
         return;
     }
     s_nvs_handles.erase(it);
+    delete static_cast<HandleEntry*>(it);
 }
 
 extern "C" esp_err_t nvs_erase_key(nvs_handle handle, const char* key)

diff --git a/components/nvs_flash/src/nvs_item_hash_list.cpp b/components/nvs_flash/src/nvs_item_hash_list.cpp
index 7fa019dffe10253ad139e8dd2cd303674127cb30..cf48477d61e354795b35a9961c885a8a49242d72 100644 (file)
--- a/components/nvs_flash/src/nvs_item_hash_list.cpp
+++ b/components/nvs_flash/src/nvs_item_hash_list.cpp
@@ -17,7 +17,11 @@
 namespace nvs
 {
 
-HashList::~HashList()
+HashList::HashList()
+{
+}
+    
+void HashList::clear()
 {
     for (auto it = mBlockList.begin(); it != mBlockList.end();) {
         auto tmp = it;
@@ -26,6 +30,11 @@ HashList::~HashList()
         delete static_cast<HashListBlock*>(tmp);
     }
 }
+    
+HashList::~HashList()
+{
+    clear();
+}
 
 HashList::HashListBlock::HashListBlock()
 {

diff --git a/components/nvs_flash/src/nvs_item_hash_list.hpp b/components/nvs_flash/src/nvs_item_hash_list.hpp
index b40a53d61554c685381a9a69dfed9bc655806a27..3f8dcc850a6dd20fd4018a6eb2e899d12e26dbed 100644 (file)
--- a/components/nvs_flash/src/nvs_item_hash_list.hpp
+++ b/components/nvs_flash/src/nvs_item_hash_list.hpp
@@ -25,11 +25,18 @@ namespace nvs
 class HashList
 {
 public:
+    HashList();
     ~HashList();
+    
     void insert(const Item& item, size_t index);
     void erase(const size_t index);
     size_t find(size_t start, const Item& item);
-
+    void clear();
+    
+private:
+    HashList(const HashList& other);
+    const HashList& operator= (const HashList& rhs);
+    
 protected:
 
     struct HashListNode {
@@ -57,7 +64,6 @@ protected:
         HashListNode mNodes[ENTRY_COUNT];
     };
 
-
     typedef intrusive_list<HashListBlock> TBlockList;
     TBlockList mBlockList;
 }; // class HashList

diff --git a/components/nvs_flash/src/nvs_page.cpp b/components/nvs_flash/src/nvs_page.cpp
index f4fc5430cdaa594375740acc908f54dd19536f21..64f7bd98523c85a3b88e1e04ad75216e521e4e62 100644 (file)
--- a/components/nvs_flash/src/nvs_page.cpp
+++ b/components/nvs_flash/src/nvs_page.cpp
@@ -741,7 +741,7 @@ esp_err_t Page::erase()
     mFirstUsedEntry = INVALID_ENTRY;
     mNextFreeEntry = INVALID_ENTRY;
     mState = PageState::UNINITIALIZED;
-    mHashList = HashList();
+    mHashList.clear();
     return ESP_OK;
 }
 

diff --git a/components/nvs_flash/test/test_nvs.cpp b/components/nvs_flash/test/test_nvs.cpp
index 3db9b45aeb27f552c0b334b3bedff2129d52d44e..ce552578db64fd426bac52c4ac9ca8f89bcf0740 100644 (file)
--- a/components/nvs_flash/test/test_nvs.cpp
+++ b/components/nvs_flash/test/test_nvs.cpp
@@ -933,6 +933,24 @@ TEST_CASE("test recovery from sudden poweroff", "[.][long][nvs][recovery][monkey
     }
 }
 
+TEST_CASE("test for memory leaks in open/set", "[leaks]")
+{
+    SpiFlashEmulator emu(10);
+    const uint32_t NVS_FLASH_SECTOR = 6;
+    const uint32_t NVS_FLASH_SECTOR_COUNT_MIN = 3;
+    emu.setBounds(NVS_FLASH_SECTOR, NVS_FLASH_SECTOR + NVS_FLASH_SECTOR_COUNT_MIN);
+    TEST_ESP_OK(nvs_flash_init_custom(NVS_FLASH_SECTOR, NVS_FLASH_SECTOR_COUNT_MIN));
+    
+    for (int i = 0; i < 100000; ++i) {
+        nvs_handle light_handle = 0;
+        char lightbulb[1024] = {12, 13, 14, 15, 16};
+        TEST_ESP_OK(nvs_open("light", NVS_READWRITE, &light_handle));
+        TEST_ESP_OK(nvs_set_blob(light_handle, "key", lightbulb, sizeof(lightbulb)));
+        TEST_ESP_OK(nvs_commit(light_handle));
+        nvs_close(light_handle);
+    }
+}
+
 TEST_CASE("dump all performance data", "[nvs]")
 {
     std::cout << "====================" << std::endl << "Dumping benchmarks" << std::endl;