search

PelionIoT / mbed-trace

mbed trace library
Apache License 2.0
18 stars 15 forks source link

Fix race condition when unlocking mutex #51

Closed kuggenhoffen closed 8 years ago

kuggenhoffen commented 8 years ago

Fixes issue where mutex might be left locked when leaving mbed_vtracef function because of decreasing mutex_lock_count after releasing the mutex.

kuggenhoffen commented 8 years ago

@kjbracey-arm @tommikas @jupe

kuggenhoffen commented 8 years ago

@TeroJaasko

kjbracey commented 8 years ago

Isn't this a bit weird anyway? Why the count?

This function claimed the mutex exactly once, so it should release it exactly once. Surely?

kjbracey commented 8 years ago

(PS welcome to the wonderful world of threading. So many really hard to figure out bugs like this...)

TeroJaasko commented 8 years ago

I guess the idea was to get the mutex from mbed_trace_array and leave it for mbed_trace_print to release. Use case would be something like this: "tr_info("hello %s", tr_array(pointer));"

tommikas commented 8 years ago

Isn't this a bit weird anyway? Why the count?

I guess the idea was to get the mutex from mbed_trace_array and leave it for mbed_trace_print to release. Use case would be something like this: "tr_info("hello %s", tr_array(pointer));"

That was exactly the reason for it. The original discussion was here: https://github.com/ARMmbed/mbed-trace/pull/30

PS welcome to the wonderful world of threading. So many really hard to figure out bugs like this...

Yeah... The implication of the post-increment in the for loop seems obvious now, but never crossed my mind back then.

teetak01 commented 8 years ago

I think they are making mbed-os 5.2 release candidate possible today so we should push this there also asap

SeppoTakalo commented 8 years ago

PR made for mbed-OS: https://github.com/ARMmbed/mbed-os/pull/2887