Merge branch 'bugfix/nvs_leaks' into 'master'

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.



See merge request !150

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)

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()
 {

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

components/nvs_flash/src/nvs_page.cpp

@@ -744,7 +744,7 @@ esp_err_t Page::erase()
     mFirstUsedEntry = INVALID_ENTRY;
     mNextFreeEntry = INVALID_ENTRY;
     mState = PageState::UNINITIALIZED;
-    mHashList = HashList();
+    mHashList.clear();
     return ESP_OK;
 }
 

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;