From 74a658c76596eed678f7595867606d2a23ca38ad Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Wed, 26 Oct 2016 12:25:53 +0800 Subject: [PATCH] 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. --- components/nvs_flash/src/nvs_api.cpp | 1 + .../nvs_flash/src/nvs_item_hash_list.cpp | 11 ++++++++++- .../nvs_flash/src/nvs_item_hash_list.hpp | 10 ++++++++-- components/nvs_flash/src/nvs_page.cpp | 2 +- components/nvs_flash/test/test_nvs.cpp | 18 ++++++++++++++++++ 5 files changed, 38 insertions(+), 4 deletions(-) diff --git a/components/nvs_flash/src/nvs_api.cpp b/components/nvs_flash/src/nvs_api.cpp index 0a56925c0..c1a910260 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(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 7fa019dff..cf48477d6 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(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 b40a53d61..3f8dcc850 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 TBlockList; TBlockList mBlockList; }; // class HashList diff --git a/components/nvs_flash/src/nvs_page.cpp b/components/nvs_flash/src/nvs_page.cpp index f4fc5430c..64f7bd985 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 3db9b45ae..ce552578d 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;