From eab643bf2d10fe604139e8560b4971eee7b07143 Mon Sep 17 00:00:00 2001
From: Nightt <87569709+nightt5879@users.noreply.github.com>
Date: Mon, 25 May 2026 10:46:59 +0800
Subject: [PATCH 1/2] system/settings: Avoid recursive save locking

Fix #3105 by making the immediate save path write pending settings while the caller still owns g_settings.mtx instead of calling the timer callback, which locks the same mutex again.

The cached-save timer callback still takes the mutex before using the same locked helper, so asynchronous saves keep their existing synchronization while non-cached saves no longer depend on a recursive mutex.

Signed-off-by: Nightt <87569709+nightt5879@users.noreply.github.com>
---
 system/settings/settings.c | 62 ++++++++++++++++++++++----------------
 1 file changed, 36 insertions(+), 26 deletions(-)

diff --git a/system/settings/settings.c b/system/settings/settings.c
index 105c093dd8e..c478e96f81c 100644
--- a/system/settings/settings.c
+++ b/system/settings/settings.c
@@ -82,7 +82,10 @@ static int      set_ip(FAR setting_t *setting, FAR struct in_addr *ip);
 static int      load(void);
 static void     save(void);
 static void     signotify(void);
+static void     dump_cache_locked(FAR bool *wrpend);
+#ifdef CONFIG_SYSTEM_SETTINGS_CACHED_SAVES
 static void     dump_cache(union sigval ptr);
+#endif
 
 /****************************************************************************
  * Private Data
@@ -623,12 +626,7 @@ static void save(void)
 #ifdef CONFIG_SYSTEM_SETTINGS_CACHED_SAVES
   timer_settime(g_settings.timerid, 0, &g_settings.trigger, NULL);
 #else
-  union sigval value =
-  {
-    .sival_ptr = &g_settings.wrpend,
-  };
-
-  dump_cache(value);
+  dump_cache_locked(&g_settings.wrpend);
 #endif
 }
 
@@ -662,51 +660,63 @@ static void signotify(void)
 }
 
 /****************************************************************************
- * Name: dump_cache
+ * Name: dump_cache_locked
  *
  * Description:
- *    Writes out the cached data to the appropriate storage
+ *    Writes out the cached data to the appropriate storage.  The caller
+ *    must hold g_settings.mtx.
  *
  * Input Parameters:
- *    value            - parameter passed if called from a timer signal
+ *    wrpend           - pending write flag to clear after dumping
  *
  * Returned Value:
  *   None
  *
  ****************************************************************************/
 
-static void dump_cache(union sigval ptr)
+static void dump_cache_locked(FAR bool *wrpend)
 {
-  int ret = OK;
-  FAR bool *wrpend = (bool *)ptr.sival_ptr;
-
   int i;
 
-  ret = pthread_mutex_lock(&g_settings.mtx);
-  assert(ret >= 0);
-
   for (i = 0; i < CONFIG_SYSTEM_SETTINGS_MAX_STORAGES; i++)
     {
       if ((g_settings.store[i].file[0] != '\0') &&
            g_settings.store[i].save_fn)
         {
-          ret = g_settings.store[i].save_fn(g_settings.store[i].file);
-#if 0
-          if (ret < 0)
-            {
-              /* What to do? We can't return anything from a void function.
-               *
-               * MIGHT BE A FUTURE REVISIT NEEDED
-               */
-            }
-#endif
+          (void)g_settings.store[i].save_fn(g_settings.store[i].file);
         }
     }
 
   *wrpend = false;
+}
 
+#ifdef CONFIG_SYSTEM_SETTINGS_CACHED_SAVES
+/****************************************************************************
+ * Name: dump_cache
+ *
+ * Description:
+ *    Writes out the cached data to the appropriate storage
+ *
+ * Input Parameters:
+ *    value            - parameter passed if called from a timer signal
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+static void dump_cache(union sigval ptr)
+{
+  FAR bool *wrpend = (FAR bool *)ptr.sival_ptr;
+  int ret;
+
+  ret = pthread_mutex_lock(&g_settings.mtx);
+  assert(ret >= 0);
+
+  dump_cache_locked(wrpend);
   pthread_mutex_unlock(&g_settings.mtx);
 }
+#endif
 
 /****************************************************************************
  * Name: sanity_check

From 948d28d7e8e6353d84cc30fb0c13dfffb38bb8c0 Mon Sep 17 00:00:00 2001
From: Nightt <87569709+nightt5879@users.noreply.github.com>
Date: Mon, 25 May 2026 10:47:43 +0800
Subject: [PATCH 2/2] system/settings: Drop unused recursive mutex type

The settings save path no longer re-enters g_settings.mtx, so the mutex does not need to be initialized as PTHREAD_MUTEX_RECURSIVE.

This keeps the #3105 fix independent of CONFIG_PTHREAD_MUTEX_TYPES and destroys the temporary mutex attribute object after initialization.

Signed-off-by: Nightt <87569709+nightt5879@users.noreply.github.com>
---
 system/settings/settings.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/system/settings/settings.c b/system/settings/settings.c
index c478e96f81c..1625c3f5a93 100644
--- a/system/settings/settings.c
+++ b/system/settings/settings.c
@@ -772,9 +772,9 @@ void settings_init(void)
   pthread_mutexattr_t attr;
 
   pthread_mutexattr_init(&attr);
-  pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE);
   pthread_mutexattr_setprotocol(&attr, PTHREAD_PRIO_INHERIT);
   pthread_mutex_init(&g_settings.mtx, &attr);
+  pthread_mutexattr_destroy(&attr);
 
   memset(map, 0, sizeof(map));
   memset(g_settings.store, 0, sizeof(g_settings.store));