Skip to content

Commit 47d0a33

Browse files
committed
fix(kernel): properly release mutexes in _thread_detach_from_mutex()
1 parent f9564d4 commit 47d0a33

File tree

2 files changed

+60
-23
lines changed

2 files changed

+60
-23
lines changed

src/ipc.c

Lines changed: 54 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1578,15 +1578,15 @@ RTM_EXPORT(rt_mutex_trytake);
15781578
* @brief This function will release a mutex. If there is thread suspended on the mutex, the thread will be resumed.
15791579
*
15801580
* @note If there are threads suspended on this mutex, the first thread in the list of this mutex object
1581-
* will be resumed, and a thread scheduling (rt_schedule) will be executed.
1581+
* will be resumed.
15821582
* If no threads are suspended on this mutex, the count value mutex->value of this mutex will increase by 1.
1583+
* This operation requires mutex->spinlock to be held.
15831584
*
15841585
* @param mutex is a pointer to a mutex object.
15851586
*
1586-
* @return Return the operation status. When the return value is RT_EOK, the operation is successful.
1587-
* If the return value is any other values, it means that the mutex release failed.
1587+
* @return Return RT_TRUE if scheduling is needed, RT_FALSE otherwise.
15881588
*/
1589-
rt_err_t rt_mutex_release(rt_mutex_t mutex)
1589+
static rt_bool_t _rt_mutex_release(rt_mutex_t mutex)
15901590
{
15911591
rt_sched_lock_level_t slvl;
15921592
struct rt_thread *thread;
@@ -1601,25 +1601,14 @@ rt_err_t rt_mutex_release(rt_mutex_t mutex)
16011601
/* only thread could release mutex because we need test the ownership */
16021602
RT_DEBUG_IN_THREAD_CONTEXT;
16031603

1604-
/* get current thread */
1605-
thread = rt_thread_self();
1606-
1607-
rt_spin_lock(&(mutex->spinlock));
1604+
/* get the current owner thread of the mutex */
1605+
thread = mutex->owner;
16081606

1609-
LOG_D("mutex_release:current thread %s, hold: %d",
1610-
thread->parent.name, mutex->hold);
1607+
LOG_D("mutex_release: mutex owner thread %s, hold count: %d",
1608+
thread->parent.name, mutex->hold);
16111609

16121610
RT_OBJECT_HOOK_CALL(rt_object_put_hook, (&(mutex->parent.parent)));
16131611

1614-
/* mutex only can be released by owner */
1615-
if (thread != mutex->owner)
1616-
{
1617-
thread->error = -RT_ERROR;
1618-
rt_spin_unlock(&(mutex->spinlock));
1619-
1620-
return -RT_ERROR;
1621-
}
1622-
16231612
/* decrease hold */
16241613
mutex->hold --;
16251614
/* if no hold */
@@ -1705,17 +1694,62 @@ rt_err_t rt_mutex_release(rt_mutex_t mutex)
17051694
}
17061695
}
17071696

1697+
return need_schedule;
1698+
}
1699+
1700+
1701+
/**
1702+
* @brief Release a mutex owned by current thread.
1703+
*
1704+
* @note Resumes first suspended thread if any (requires scheduling).
1705+
* Increases mutex->value if no threads waiting.
1706+
* Must be called by mutex owner with spinlock held.
1707+
*
1708+
* @param mutex Pointer to the mutex object.
1709+
*
1710+
* @return RT_EOK on success, -RT_ERROR if not owner.
1711+
*/
1712+
rt_err_t rt_mutex_release(rt_mutex_t mutex)
1713+
{
1714+
struct rt_thread *thread;
1715+
rt_bool_t need_schedule;
1716+
1717+
/* parameter check */
1718+
RT_ASSERT(mutex != RT_NULL);
1719+
RT_ASSERT(rt_object_get_type(&mutex->parent.parent) == RT_Object_Class_Mutex);
1720+
1721+
need_schedule = RT_FALSE;
1722+
1723+
/* only thread could release mutex because we need test the ownership */
1724+
RT_DEBUG_IN_THREAD_CONTEXT;
1725+
1726+
/* get current thread */
1727+
thread = rt_thread_self();
1728+
1729+
rt_spin_lock(&(mutex->spinlock));
1730+
1731+
if (thread != mutex->owner)
1732+
{
1733+
thread->error = -RT_ERROR;
1734+
rt_spin_unlock(&(mutex->spinlock));
1735+
1736+
return -RT_ERROR;
1737+
}
1738+
1739+
need_schedule= _rt_mutex_release(mutex);
1740+
17081741
rt_spin_unlock(&(mutex->spinlock));
17091742

17101743
/* perform a schedule */
17111744
if (need_schedule == RT_TRUE)
1745+
{
17121746
rt_schedule();
1747+
}
17131748

17141749
return RT_EOK;
17151750
}
17161751
RTM_EXPORT(rt_mutex_release);
17171752

1718-
17191753
/**
17201754
* @brief This function will set some extra attributions of a mutex object.
17211755
*

src/thread.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,8 @@ RT_OBJECT_HOOKLIST_DEFINE(rt_thread_inited);
7777
#endif /* defined(RT_USING_HOOK) && defined(RT_HOOK_USING_FUNC_PTR) */
7878

7979
#ifdef RT_USING_MUTEX
80+
rt_bool_t _rt_mutex_release(rt_mutex_t mutex);
81+
8082
static void _thread_detach_from_mutex(rt_thread_t thread)
8183
{
8284
rt_list_t *node;
@@ -101,9 +103,10 @@ static void _thread_detach_from_mutex(rt_thread_t thread)
101103
{
102104
mutex = rt_list_entry(node, struct rt_mutex, taken_list);
103105
LOG_D("Thread [%s] exits while holding mutex [%s].\n", thread->parent.name, mutex->parent.parent.name);
104-
/* recursively take */
105-
mutex->hold = 1;
106-
rt_mutex_release(mutex);
106+
rt_spin_lock(&(mutex->spinlock));
107+
mutex->hold=1;
108+
_rt_mutex_release(mutex);
109+
rt_spin_unlock(&(mutex->spinlock));
107110
}
108111

109112
rt_spin_unlock_irqrestore(&thread->spinlock, level);

0 commit comments

Comments
 (0)