From: Ivan Grokhotkov <ivan@espressif.com>
Date: Wed, 26 Oct 2016 04:25:53 +0000 (+0800)
Subject: nvs: fix memory leaks in HashList and nvs_close
X-Git-Tag: v1.0~93^2
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=74a658c76596eed678f7595867606d2a23ca38ad;p=esp-idf

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 0a56925c0c..c1a910260e 100644
--- 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 7fa019dffe..cf48477d61 100644
--- 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 b40a53d615..3f8dcc850a 100644
--- 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 f4fc5430cd..64f7bd9852 100644
--- 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 3db9b45aeb..ce552578db 100644
--- 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;