Mutex around writer access in recorder

[Stapelzeiger]

The storage plugins are not designed to be thread safe. The recorder must therefore use a mutex when accessing the writer from different contexts. This happens for topic creation during discovery as well as in message callbacks from subscribed topics.

This fixes #486 and #480.

[Karsten1987]

Thanks for kicking this off. I am requesting changes here though for two reasons:

• I am not sure if rosbag2_transport is the right location to protect for concurrency. While this would certainly help for the command line interface, I think the right place is within rosbag2_cpp.
• Call it premature optimization (I have to measure my claim first), but I would expect this to lead to other performance issues as the actual call to write can be blocked if (creating a subscription takes too long or many subscriptions are being created in a row). I am not 100% certain yet what the best way is but I would like to see the call to write basically being wait free.

[Stapelzeiger]

@Karsten1987 I added locking in the rosbag2_cpp writer. I've reverted the changes in recorder, since they are redundant with the rosbag2_cpp ones so this should solve your second point regarding the create_subscription in the critical zone.

Do we need a similar thread-safe interface for the reader class? I have trouble imagining a use-case where you would read the from multiple threads with the current API (in particular the has_next(), read_next() wouldn't work nicely).

[Karsten1987]

I think this looks good to me. @emersonknapp any thoughts on this?

@Stapelzeiger please sign your commits to make the DCO bot happy.

[jarledo]

Any updates on this? Our system suffers from the bug described in #486, making it almost impossible to record our topics. Would be great to have this merged.

[Karsten1987]

Thanks for the reminder, kicking off CI on this:

• Linux
• Linux-aarch64
• macOS
• Windows

[Karsten1987]

@Stapelzeiger thanks for the contribution

[Karsten1987]

@adamdbrw fyi

[almaslov]

Hello! It would be great to backport this fix to Foxy. Can I do it myself according to the Guide ?

[emersonknapp]

Yes, by all means. If the cherry-pick works without issue then it is an easy backport.