-
Notifications
You must be signed in to change notification settings - Fork 5.3k
fix(kernel): properly release mutexes in _thread_detach_from_mutex() #10545
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1575,18 +1575,32 @@ 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. | ||||||||||||||||
| * If no threads are suspended on this mutex, the count value mutex->value of this mutex will increase by 1. | ||||||||||||||||
| * | ||||||||||||||||
| * @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. | ||||||||||||||||
| * @brief Release a mutex, optionally forcing the release even if the current thread is not the owner. | ||||||||||||||||
| * If threads are suspended on the mutex, the highest-priority thread will be resumed. | ||||||||||||||||
| * | ||||||||||||||||
| * @note This function provides two distinct release modes: | ||||||||||||||||
| * Standard mode enforces normal ownership rules where only the owning thread | ||||||||||||||||
| * may release the mutex. Force mode bypasses ownership checks and directly | ||||||||||||||||
| * modifies the hold count to enable release by any thread. | ||||||||||||||||
| * | ||||||||||||||||
| * In both modes, if threads are suspended on the mutex, the highest priority | ||||||||||||||||
| * waiting thread will be resumed, potentially triggering thread scheduling. | ||||||||||||||||
| * When no threads are waiting, the mutex owner and hold count are reset. | ||||||||||||||||
| * | ||||||||||||||||
| * @param mutex Pointer to the mutex object. | ||||||||||||||||
| * @param is_force If RT_TRUE, bypass ownership check and force release. | ||||||||||||||||
| * If RT_FALSE, enforce standard owner-only release. | ||||||||||||||||
| * | ||||||||||||||||
| * @return Operation status: | ||||||||||||||||
| * RT_EOK: Success. | ||||||||||||||||
| * -RT_ERROR: Failed (non-force mode and current thread is not owner). | ||||||||||||||||
| * | ||||||||||||||||
| * @warning Forced release (is_force=RT_TRUE) should only be used when: | ||||||||||||||||
| * The caller is not the mutex owner and the original owner thread | ||||||||||||||||
| * is guaranteed to be closed (rt_thread_close) and no longer executing any code, | ||||||||||||||||
| * or the caller is the current mutex owner. | ||||||||||||||||
| */ | ||||||||||||||||
| rt_err_t rt_mutex_release(rt_mutex_t mutex) | ||||||||||||||||
| static rt_err_t _rt_mutex_release(rt_mutex_t mutex, rt_bool_t is_force) | ||||||||||||||||
| { | ||||||||||||||||
| rt_sched_lock_level_t slvl; | ||||||||||||||||
| struct rt_thread *thread; | ||||||||||||||||
|
|
@@ -1611,9 +1625,21 @@ rt_err_t rt_mutex_release(rt_mutex_t mutex) | |||||||||||||||
|
|
||||||||||||||||
| RT_OBJECT_HOOK_CALL(rt_object_put_hook, (&(mutex->parent.parent))); | ||||||||||||||||
|
|
||||||||||||||||
| /* mutex only can be released by owner */ | ||||||||||||||||
| if (thread != mutex->owner) | ||||||||||||||||
| if (is_force) | ||||||||||||||||
| { | ||||||||||||||||
| /* | ||||||||||||||||
| * Force release mode: | ||||||||||||||||
| * Bypass ownership check and prepare for release by | ||||||||||||||||
| * setting hold count to 1 before decrement | ||||||||||||||||
| */ | ||||||||||||||||
| mutex->hold = 1; | ||||||||||||||||
| } | ||||||||||||||||
| else if (thread != mutex->owner) | ||||||||||||||||
| { | ||||||||||||||||
| /* | ||||||||||||||||
| * Standard release mode: | ||||||||||||||||
| * Reject non-owner thread's release attempt | ||||||||||||||||
| */ | ||||||||||||||||
| thread->error = -RT_ERROR; | ||||||||||||||||
| rt_spin_unlock(&(mutex->spinlock)); | ||||||||||||||||
|
|
||||||||||||||||
|
|
@@ -1713,9 +1739,47 @@ rt_err_t rt_mutex_release(rt_mutex_t mutex) | |||||||||||||||
|
|
||||||||||||||||
| return RT_EOK; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| /** | ||||||||||||||||
| * @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) | ||||||||||||||||
| { | ||||||||||||||||
| return _rt_mutex_release(mutex, RT_FALSE); | ||||||||||||||||
| } | ||||||||||||||||
| RTM_EXPORT(rt_mutex_release); | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| /** | ||||||||||||||||
| * @brief Forcefully release a mutex regardless of ownership or recursive holds. | ||||||||||||||||
| * | ||||||||||||||||
| * @note This function bypasses all ownership verification and ignores recursive hold counts. | ||||||||||||||||
| * It will resume the first suspended thread if available, which may trigger scheduling. | ||||||||||||||||
| * If no threads are waiting, the mutex value will be reset. | ||||||||||||||||
| * | ||||||||||||||||
| * @warning When releasing a mutex not owned by the caller, the original owner thread | ||||||||||||||||
| * must have been properly terminated via rt_thread_close and must not be | ||||||||||||||||
| * executing any code at all. | ||||||||||||||||
|
Comment on lines
+1769
to
+1771
|
||||||||||||||||
| * @warning When releasing a mutex not owned by the caller, the original owner thread | |
| * must have been properly terminated via rt_thread_close and must not be | |
| * executing any code at all. | |
| * @warning Forced release bypasses recursive hold counts and may leave the system in an inconsistent state | |
| * if the original owner had taken the mutex multiple times recursively. When releasing a mutex not | |
| * owned by the caller, the original owner thread must have been properly terminated via rt_thread_close | |
| * and must not be executing any code at all. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The warning documentation should specify that forced release bypasses the recursive mutex mechanism, which could lead to undefined behavior if the original owner thread is still running and expects to maintain its hold count.