fix(kernel): properly release mutexes in _thread_detach_from_mutex()

Author: eatvector

include/rtthread.h

@@ -465,6 +465,7 @@ rt_err_t rt_mutex_trytake(rt_mutex_t mutex);
 rt_err_t rt_mutex_take_interruptible(rt_mutex_t mutex, rt_int32_t time);
 rt_err_t rt_mutex_take_killable(rt_mutex_t mutex, rt_int32_t time);
 rt_err_t rt_mutex_release(rt_mutex_t mutex);
+rt_bool_t _rt_mutex_release_detach(rt_mutex_t mutex);
 rt_err_t rt_mutex_control(rt_mutex_t mutex, int cmd, void *arg);
 
 rt_inline rt_thread_t rt_mutex_get_owner(rt_mutex_t mutex)

src/ipc.c

@@ -1578,15 +1578,15 @@ RTM_EXPORT(rt_mutex_trytake);
  * @brief    This function will release a mutex. If there is thread suspended on the mutex, the thread will be resumed.
  *
  * @note     If there are threads suspended on this mutex, the first thread in the list of this mutex object
- *           will be resumed, and a thread scheduling (rt_schedule) will be executed.
+ *           will be resumed.
  *           If no threads are suspended on this mutex, the count value mutex->value of this mutex will increase by 1.
+ *           This operation requires mutex->spinlock to be held.
  *
  * @param    mutex is a pointer to a mutex object.
  *
- * @return   Return the operation status. When the return value is RT_EOK, the operation is successful.
- *           If the return value is any other values, it means that the mutex release failed.
+ * @return   Return RT_TRUE if scheduling is needed, RT_FALSE otherwise.
  */
-rt_err_t rt_mutex_release(rt_mutex_t mutex)
+rt_bool_t _rt_mutex_release(rt_mutex_t mutex)
 {
     rt_sched_lock_level_t slvl;
     struct rt_thread *thread;
@@ -1601,25 +1601,14 @@ rt_err_t rt_mutex_release(rt_mutex_t mutex)
     /* only thread could release mutex because we need test the ownership */
     RT_DEBUG_IN_THREAD_CONTEXT;
 
-    /* get current thread */
-    thread = rt_thread_self();
-
-    rt_spin_lock(&(mutex->spinlock));
+    /* get the current owner thread of the mutex */
+    thread = mutex->owner;
 
-    LOG_D("mutex_release:current thread %s, hold: %d",
-          thread->parent.name, mutex->hold);
+    LOG_D("mutex_release: mutex owner thread %s, hold count: %d",
+      thread->parent.name, mutex->hold);
 
     RT_OBJECT_HOOK_CALL(rt_object_put_hook, (&(mutex->parent.parent)));
 
-    /* mutex only can be released by owner */
-    if (thread != mutex->owner)
-    {
-        thread->error = -RT_ERROR;
-        rt_spin_unlock(&(mutex->spinlock));
-
-        return -RT_ERROR;
-    }
-
     /* decrease hold */
     mutex->hold --;
     /* if no hold */
@@ -1705,17 +1694,68 @@ rt_err_t rt_mutex_release(rt_mutex_t mutex)
         }
     }
 
+    return need_schedule;
+}
+
+
+rt_bool_t _rt_mutex_release_detach(rt_mutex_t mutex)
+{
+    _rt_mutex_release(mutex);
+}
+
+
+/**
+ * @brief    Release a mutex owned by current thread.
+ *
+ * @note     Resumes first suspended thread if any (requires scheduling).
+ *           Increases mutex->value if no threads waiting.
+ *           Must be called by mutex owner with spinlock held.
+ *
+ * @param    mutex Pointer to the mutex object.
+ *
+ * @return   RT_EOK on success, -RT_ERROR if not owner.
+ */
+rt_err_t rt_mutex_release(rt_mutex_t mutex)
+{
+    struct rt_thread *thread;
+    rt_bool_t need_schedule;
+
+    /* parameter check */
+    RT_ASSERT(mutex != RT_NULL);
+    RT_ASSERT(rt_object_get_type(&mutex->parent.parent) == RT_Object_Class_Mutex);
+
+    need_schedule = RT_FALSE;
+
+    /* only thread could release mutex because we need test the ownership */
+    RT_DEBUG_IN_THREAD_CONTEXT;
+
+    /* get current thread */
+    thread = rt_thread_self();
+
+    rt_spin_lock(&(mutex->spinlock));
+
+    if (thread != mutex->owner)
+    {
+        thread->error = -RT_ERROR;
+        rt_spin_unlock(&(mutex->spinlock));
+
+        return -RT_ERROR;
+    }
+
+    need_schedule= _rt_mutex_release(mutex);
+
     rt_spin_unlock(&(mutex->spinlock));
 
     /* perform a schedule */
     if (need_schedule == RT_TRUE)
+    {
         rt_schedule();
+    }
 
     return RT_EOK;
 }
 RTM_EXPORT(rt_mutex_release);
 
-
 /**
  * @brief    This function will set some extra attributions of a mutex object.
  *

src/thread.c

@@ -77,6 +77,7 @@ RT_OBJECT_HOOKLIST_DEFINE(rt_thread_inited);
 #endif /* defined(RT_USING_HOOK) && defined(RT_HOOK_USING_FUNC_PTR) */
 
 #ifdef RT_USING_MUTEX
+
 static void _thread_detach_from_mutex(rt_thread_t thread)
 {
     rt_list_t *node;
@@ -101,9 +102,10 @@ static void _thread_detach_from_mutex(rt_thread_t thread)
     {
         mutex = rt_list_entry(node, struct rt_mutex, taken_list);
         LOG_D("Thread [%s] exits while holding mutex [%s].\n", thread->parent.name, mutex->parent.parent.name);
-        /* recursively take */
-        mutex->hold = 1;
-        rt_mutex_release(mutex);
+        rt_spin_lock(&(mutex->spinlock));
+        mutex->hold=1;
+        _rt_mutex_release(mutex);
+        rt_spin_unlock(&(mutex->spinlock));
     }
 
     rt_spin_unlock_irqrestore(&thread->spinlock, level);