sys/xtimer/xtimer.c: _mutex_timeout() bug fix #11992
Merged
Conversation
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
de321cb to
e6e0346
Compare
6 tasks
|
rebase because of #12008 got merged |
e6e0346 to
0a3827d
Compare
There was a problem hiding this comment.
Finally found the time to test this one. I could reproduce the bug and verify that the fix is working. Nice one.
After struggling a bit to understand how the mutex/queue handling works here and what is implied by that, I wonder If we should just add some comments explaining what happens there to make it easier for the next one that looks at this code.
|
@MichelRottleuthner I will create a function for removing a thread from a mutex list ( this is what the |
Sure go ahead. |
Prevent a possible race condition when _mutex_timeout fires just after the
mutex was locked but before the xtimer was removed
The flow
int xtimer_mutex_lock_timeout(mutex_t *mutex, uint64_t timeout) {
...
mutex_lock(mutex);
/* mutex locked */
/* _mutex_timeout fires and tries to remove thread from mutex queue */
/* DEBUG: simulate callback call between lock and remove */
xtimer_spin(xtimer_ticks_from_usec(timeout*2));
xtimer_remove(&t);
...
}
no longer into the "if" when the mutex is free
0a3827d to
7cd3e8b
Compare
|
@MichelRottleuthner I rebased. And removed REMOVE_ME commits |
MichelRottleuthner
approved these changes
Dec 4, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
HAS "REMOVE ME" COMMIT TO SHOW BUG
This includes pr: #10872
Tracking: pr #11660
Contribution description
Fixes BUG.
The function
_mutex_timeoutremoves a thread from the waiting list of a mutex and unblocks the thread (sets status toSTATUS_PENDING) when the thread is waiting for the mutex. This is used forxtimer_mutex_lock_timeouta mutex lock function with timeout. This pr fixes a bug that happens because the function_mutex_timeoutnever checks if the mutex has a waiting list. This pr adds this check.When the Timer triggers after the mutex is acquired but before the timer is removed.. You can see the bug with the "REMOVE ME" commit (asserts when the mutex is acquired and the timer triggers after).
The Function (
_mutex_timeout) does nothing when the thread got the mutex (and no thread is waiting for itmutex->queue.next == MUTEX_LOCKED) and when the mutex is free (mutex->queue.next == NULL).list_removeshould not be called with a mutex wheremutex->queue.next == MUTEX_LOCKED.Testing procedure
BOARD=native make -C tests/xtimer_mutex_lock_timeout/ flash testoutput:
Revert the
Revert "REMOVE ME"commit to see it work after fix.output:
Revert to
REMOVE MEcommit and run the test to see the bug.output:
Issues/PRs references
Depends on PR #11807
Tracking PR #11660
Includes PR #10872
my old pr fixing same bug: #11543