fix(nvs): Improved lockig mechanism for initialization phase

components/nvs_flash/CMakeLists.txt

@@ -11,10 +11,12 @@ set(srcs "src/nvs_api.cpp"
          "src/nvs_partition.cpp"
          "src/nvs_partition_lookup.cpp"
          "src/nvs_partition_manager.cpp"
-         "src/nvs_types.cpp")
+         "src/nvs_types.cpp"
+         "src/nvs_platform.cpp")
 
 idf_component_register(SRCS "${srcs}"
-                    REQUIRES "spi_flash"
+                    REQUIRES "esp_partition"
+                    PRIV_REQUIRES spi_flash newlib
                     INCLUDE_DIRS "include"
                     PRIV_INCLUDE_DIRS "private_include")
 

components/nvs_flash/src/nvs_api.cpp

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -45,10 +45,6 @@ uint32_t NVSHandleEntry::s_nvs_next_handle;
 
 extern "C" void nvs_dump(const char *partName);
 
-#ifndef LINUX_TARGET
-SemaphoreHandle_t nvs::Lock::mSemaphore = nullptr;
-#endif // ! LINUX_TARGET
-
 using namespace std;
 using namespace nvs;
 
@@ -272,6 +268,10 @@ static esp_err_t nvs_find_ns_handle(nvs_handle_t c_handle, NVSHandleSimple** han
 
 extern "C" esp_err_t nvs_open_from_partition(const char *part_name, const char* name, nvs_open_mode_t open_mode, nvs_handle_t *out_handle)
 {
+    esp_err_t lock_result = Lock::init();
+    if (lock_result != ESP_OK) {
+        return lock_result;
+    }
     Lock lock;
     ESP_LOGD(TAG, "%s %s %d", __func__, name, open_mode);
 

components/nvs_flash/src/nvs_platform.cpp

@@ -0,0 +1,50 @@
+/*
+ * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+#include "nvs_platform.hpp"
+
+using namespace nvs;
+
+#ifdef LINUX_TARGET
+Lock::Lock() {}
+Lock::~Lock() {}
+esp_err_t nvs::Lock::init() {return ESP_OK;}
+void Lock::uninit() {}
+#else
+
+#include "sys/lock.h"
+
+Lock::Lock()
+{
+    // Newlib implementation ensures that even if mSemaphore was 0, it gets initialized.
+    // Locks mSemaphore
+    _lock_acquire(&mSemaphore);
+}
+
+Lock::~Lock()
+{
+    // Unlocks mSemaphore
+    _lock_release(&mSemaphore);
+}
+
+esp_err_t Lock::init()
+{
+    // Let postpone initialization to the Lock::Lock.
+    // It is designed to lazy initialize the semaphore in a properly guarded critical section
+    return ESP_OK;
+}
+
+void Lock::uninit()
+{
+    // Uninitializes mSemaphore. Please be aware that uninitialization of semaphore shared and held by another thread
+    // can cause undefined behavior
+    if (mSemaphore) {
+        _lock_close(&mSemaphore);
+    }
+}
+
+_lock_t Lock::mSemaphore = 0;
+
+#endif

components/nvs_flash/src/nvs_platform.hpp

@@ -1,82 +1,24 @@
-// Copyright 2015-2016 Espressif Systems (Shanghai) PTE LTD
+/*
-//
+ * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD
-// Licensed under the Apache License, Version 2.0 (the "License");
+ *
-// you may not use this file except in compliance with the License.
+ * SPDX-License-Identifier: Apache-2.0
-// You may obtain a copy of the License at
+ */
-
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
 #pragma once
 
-#ifdef LINUX_TARGET
+#include "esp_err.h"
-namespace nvs
-{
-class Lock
-{
-public:
-    Lock() { }
-    ~Lock() { }
-    static esp_err_t init()
-    {
-        return ESP_OK;
-    }
-
-    static void uninit() {}
-};
-} // namespace nvs
-
-#else // LINUX_TARGET
-
-#include "freertos/FreeRTOS.h"
-#include "freertos/semphr.h"
 
 namespace nvs
 {
-
+    class Lock
-class Lock
-{
-public:
-    Lock()
     {
-        if (mSemaphore) {
+    public:
-            xSemaphoreTake(mSemaphore, portMAX_DELAY);
+        Lock();
-        }
+        ~Lock();
-    }
+        static esp_err_t init();
-
+        static void uninit();
-    ~Lock()
+#ifndef LINUX_TARGET
-    {
+    private:
-        if (mSemaphore) {
+        static _lock_t mSemaphore;
-            xSemaphoreGive(mSemaphore);
+#endif
-        }
+    };
-    }
-
-    static esp_err_t init()
-    {
-        if (mSemaphore) {
-            return ESP_OK;
-        }
-        mSemaphore = xSemaphoreCreateMutex();
-        if (!mSemaphore) {
-            return ESP_ERR_NO_MEM;
-        }
-        return ESP_OK;
-    }
-
-    static void uninit()
-    {
-        if (mSemaphore) {
-            vSemaphoreDelete(mSemaphore);
-        }
-        mSemaphore = nullptr;
-    }
-
-    static SemaphoreHandle_t mSemaphore;
-};
 } // namespace nvs
-
-#endif // LINUX_TARGET

components/nvs_flash/src/nvs_storage.cpp

@@ -204,7 +204,6 @@ esp_err_t Storage::init(uint32_t baseSector, uint32_t sectorCount)
     if (mNamespaceUsage.set(255, true) != ESP_OK) {
         return ESP_FAIL;
     }
-    mState = StorageState::ACTIVE;
 
     // Populate list of multi-page index entries.
     TBlobIndexList blobIdxList;
@@ -223,6 +222,8 @@ esp_err_t Storage::init(uint32_t baseSector, uint32_t sectorCount)
     // Purge the blob index list
     blobIdxList.clearAndFreeNodes();
 
+    mState = StorageState::ACTIVE;
+
 #ifdef DEBUG_STORAGE
     debugCheck();
 #endif

components/nvs_flash/test_nvs_host/Makefile

@@ -16,6 +16,7 @@ SOURCE_FILES = \
 		nvs_partition.cpp \
 		nvs_encrypted_partition.cpp \
 		nvs_cxx_api.cpp \
+		nvs_platform.cpp \
 	) \
 	spi_flash_emulation.cpp \
 	test_compressed_enum_table.cpp \