Skip to content

xtimer: check in xtimer_mutex_lock_timeout timeout callback if a thread blocked on mutex #10872

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

xtimer: check in xtimer_mutex_lock_timeout timeout callback if a thread blocked on mutex #10872

wants to merge 1 commit into from

Conversation

pwillem
Copy link

@pwillem pwillem commented Jan 25, 2019

Contribution description

Prevent a possible race condition for xtimer_mutex_lock_timeout 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 ->
    xtimer_remove(&t);
    ...
}

Testing procedure

I didn't try to reproduce the bug because a backtrace was provided

gdb) bt#0  0x080558e2 in list_remove (list=0xffffffff, node=0x20008094 <_stack+1912>) at /RIOT/core/include/list.h:88
#1  0x08055a7a in _mutex_timeout (arg=0x20007f68 <_stack+1612>) at /RIOT/sys/xtimer/xtimer.c:241
#2  0x0800375c in _shoot (timer=0x20007f74 <_stack+1624>) at /RIOT/sys/xtimer/xtimer_core.c:166
#3  0x08003c36 in _timer_callback () at /RIOT/sys/xtimer/xtimer_core.c:517
#4  0x08003740 in _periph_timer_callback (arg=0x0, chan=0) at /RIOT/sys/xtimer/xtimer_core.c:161
#5  0x08004a74 in irq_handler (tim=0) at /RIOT/cpu/stm32_common/periph/timer.c:116
#6  0x08004a9a in isr_tim5 () at /RIOT/cpu/stm32_common/periph/timer.c:125
#7  <signal handler called>
#8  0x080010de in cpu_switch_context_exit () at /RIOT/cpu/cortexm_common/thread_arch.c:266
Backtrace stopped: previous frame identical to this frame (corrupt stack?)

gdb) bt#0  0x080010f8 in thread_yield_higher () at /RIOT/cpu/cortexm_common/thread_arch.c:280
#1  0x08000afc in _mutex_lock (mutex=0x2000a1f0 <_rsp_available>, blocking=1) at /RIOT/core/mutex.c:62
#2  0x08055908 in mutex_lock (mutex=0x2000a1f0 <_rsp_available>) at /RIOT/core/include/mutex.h:113
#3  0x08055b02 in xtimer_mutex_lock_timeout (mutex=0x2000a1f0 <_rsp_available>, timeout=500000) at /RIOT/sys/xtimer/xtimer.c:261

Issues/PRs references

Didn't create an issues

xtimer: check in timeout callback if thread blocked on mutex
d5c98d1 
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 ->
    xtimer_remove(&t);
    ...
}
@miri64 miri64 requested a review from vincent-d January 25, 2019 19:35
@miri64 miri64 added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: timers Area: timer subsystems labels Jan 25, 2019
@miri64 miri64 added this to the Release 2019.01 milestone Jan 25, 2019
@vincent-d
Copy link
Member

I just remembered that there was another PR which has been opened for quite some time now which may be better than this one because it covers more corner cases: #6441.

Maybe we should try to have it in instead of this one.

@vincent-d vincent-d mentioned this pull request Jan 28, 2019
Closed
@JulianHolzwarth JulianHolzwarth mentioned this pull request May 17, 2019
Closed
@JulianHolzwarth JulianHolzwarth mentioned this pull request Aug 9, 2019
Merged
@kb2ma kb2ma removed this from the Release 2019.10 milestone Oct 8, 2019
@MichelRottleuthner
Copy link
Contributor

Closing in favor of #11992 (which also contains the changes from this PR)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vincent-d vincent-d Awaiting requested review from vincent-d

Assignees
No one assigned
Labels
Area: timers Area: timer subsystems Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@pwillem @vincent-d @MichelRottleuthner @miri64 @aabadie @kb2ma @MrKevinWeiss