Fix race condition in test clock

[pedroazeredo04]

This Pull Request intends to fix the issue #2804

In order to do so, I first ran helgrind on the code, and found that there existed a race condition in a vector of booleans. Afterwards, I changed this vector to std::atomic_bool . With that, I ran the helgrind again, and found there was no more race condition.

This is the full log of helgrind for the original code.
This is the full log of helgrind for the modified code.

Note that, since they are the full log of the helgrind, in both of these snippets, there are some unrelated warnings. However, by searching for data race inside this logs, it is possible to see that they were corrected.

This is my first Pull Request, so if there are any changes I still have to do, please let me know.
(Edit: I just realized my signature in the commit was wrong. I believe I have corrected it, hopefully everything is OK now)

[fujitatomoya]

lgtm.

@pedroazeredo04 thank you very much for creating PR. can you retarget this PR against rolling branch? that is our development branch where all fixes are supposed to be merged in rolling, and then we can backport the fixes to released distro such as jazzy?

[pedroazeredo04]

@fujitatomoya

So, I retargeted this PR to the rolling branch, but it seems like there are some conflicts because my original branch was based on jazzy. Should I do a rebase, or would it be better to create a new branch and a new PR?

[fujitatomoya]

@Mergifyio rebase

[mergify]

rebase

☑️ Nothing to do

• -conflict [📌 rebase requirement]
• -closed [📌 rebase requirement]
• queue-position = -1 [📌 rebase requirement]
• any of:
  • #commits > 1 [📌 rebase requirement]
  • #commits-behind > 0 [📌 rebase requirement]
  • -linear-history [📌 rebase requirement]

[fujitatomoya]

@pedroazeredo04 i tried to rebase by Mergifyio, but it did not work. yes, please rebase this to rolling branch.

[jmachowinski]

I recommend not doing a rebase, but instead cherry pick your commit on rolling. Afterwards force push to your branch for this pr.

[pedroazeredo04]

Nice, thanks a lot @fujitatomoya and @jmachowinski. At the end, I did the cherry pick and force push, it looks like it worked. I was a bit scared about the rebase, there were a lot of conflicts and I was worried about messing up something ahahaha

[jmachowinski]

Pulls: #2819
Gist: https://gist.githubusercontent.com/jmachowinski/3ccac15337693ffaf740c9e338fc3912/raw/d459b0ee51366d0ba3086e2b4743e9b7b48c536f/ros2.repos
BUILD args:
TEST args:
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/15716

• Linux
• Linux-aarch64
• Linux-rhel
• Windows

[jmachowinski]

Test failure on Linux is unrelated

[jmachowinski]

Thanks for the PR @pedroazeredo04

[fujitatomoya]

@jmachowinski we are currently branching on freeze...?

[fujitatomoya]

@Mergifyio backport jazzy

[mergify]

backport jazzy

✅ Backports have been created

• #2821 Fix race condition in test clock (backport #2819) has been created for branch jazzy

[jmachowinski]

@jmachowinski we are currently branching on freeze...?

There was a message the rolling sync was done. Are we still on freeze?

[christophebedard]

We're still in the freeze. The Rolling sync is for binary releases (related to rosdistro).

[christophebedard]

That being said, this only changes a test and does fix an issue, so it's probably fine!