PelionIoT / mbed-trace

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

Added mutex wait/release callback hooks for thread safety. #30

Closed tommikas closed 8 years ago

tommikas commented 8 years ago

Also added very simple unit tests and added yotta build/test generated stuff to .mbedignore.

tommikas commented 8 years ago

@jupe @teetak01 @SeppoTakalo Check this please.

tommikas commented 8 years ago

Usage example pending push access so I can push and create a PR to link.

jupe commented 8 years ago

please update readme as well

jupe commented 8 years ago

those helper functions needs also protection so that buffers doesnt mess when calling from threads..

tommikas commented 8 years ago

Good point... At first glance I thought this was enough since those helpers are always called from within the trace call, but of course the helpers actually get called first.

tommikas commented 8 years ago

Putting locks in the helper functions still leaves the possibility of another thread acquiring the lock in between the helper function releasing it and the actual trace function acquiring it, if the thread gets preempted.

Having the lock in place from first entering into the first called helper function until returning from the trace call itself would prevent that. One way to achieve that would be to use a counting mutex that is acquired but not released in helper functions, and then completely released before returning from the actual trace call. Unfortunately that means needing to have recursive mutexes available, instead of just a simple mutex.

tommikas commented 8 years ago

Also changed relevant things brought up in ARMmbed/mbed-client-cli#42.

kjbracey commented 8 years ago

The lock thing's a bit scary, but can't see a better alternative. Does mean that an external buffer-sharing "helper" would need API to grab that.

Feels a bit like the mutex should be implicit in tmp_data_left(), or explicitly acquired by some sort of tmp_data-type call, that external helpers would get access to.

tommikas commented 8 years ago

Adding acquiring the lock into tmp_data_left() as it is would ensure that it gets done whenever actually needed, as long as tmp_data_left() is used correctly. The name of the macro would seem ill-fitting with that baked in though.

From the point of view of the hypothetical external helper API it'd simplify things though - a single call that locks if needed and returns number of bytes left in the buffer before starting to do anything else. Of course such a call and the current implementation internal to mbed-trace wouldn't be mutually exclusive, so it'd be easy to add whenever.

teetak01 commented 8 years ago

+1

jupe commented 8 years ago

+1