Fix race condition in transport recorder

[carlossvg]

This PR fixes a race condition when creating a topic in robasg2 transport recorder.

Sometimes test_rosbag2_record_end_to_end fails by timeout with the following error messages:

1: [ERROR] [rosbag2_transport]: Failed to record: Topic '/test_topic' has not been created yet! Call 'create_topic' first

Signed-off-by: Carlos San Vicente carlos.sanvicente@apex.ai

[emersonknapp]

Thanks for the contribution!

Can you think of a way to add a regression test for this fix?

Also, please a comment in the code for why this order of operations is necessary.

[MichaelOrlov]

Hi Emerson @emersonknapp,
I am actually author of this fix in Apex.AI.
What kind of regression tests you are requesting? Can you please elaborate?
This is fix for race condition. I am kind of confused about your request. If somebody will try to add some unit test to make sure that there no race conditions it will be non deterministic and will fail from time to time or test nothing.
If you have idea how to properly write such a test - please let us know.

As regarding to explanation of why this order of operation is necessary:
We need to create topic in writer before we are trying to create subscription. Since in callback for subscription we are calling writer_->write(bag_message); and it could happened that callback called before we reached out the line: writer_->create_topic(topic)

[emersonknapp]

If it's too hard then that's ok, but I want to make sure we've always at least thought through regression tests for fixes, and it's good to note that thought process out in the open either way.

Can you please add that comment to the code itself? That way the next implementer to change that is better informed.

[MichaelOrlov]

Sure, we will add comments about it in code.

[MichaelOrlov]

Hi @emersonknapp Can you please kindly review/approve this changes. I've added comment about importance of order of operations.

[emersonknapp]

Looks good! Thanks for pinging me again, I can lose updates sometimes