Bugfix/rclcpp action UUID rng race condition

[iv461]

Description

Fixes #3182

Is this user-facing behavior change?

No

Did you use Generative AI?

No :)

Additional Information

[mergify]

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

• Queue this pull request

[fujitatomoya]

and why dose this need to be recursive mutex?

[iv461]

It does not need to be, but it prevents deadlocks while having no other disadvantages. So wheneever I'm unsure if there would be an reentrant call to lock from the same thread in the future (if the code changes, gets refactored etc.), I use a reentrant mutex instead of a regular one.

[jmachowinski]

There is a slight overhead from recursive mutexes. In this case I don't see a deadlock ever happen therefore I would use a normal mutex.

[iv461]

Yes, there is a slight, but insignificant (100 nanoseconds ?) overhead. Therefore I think it is more sensible to just use the reentrant mutex.

[jmachowinski]

The usage of a reentrant mutex is not reasonable in this case, the lock part is VERY small. Also putting in something that is sub optimal because something can change in the future is not a good argument. This leads to code bloat.

If a future commit manages to add some sort of recursion to this function (which I doubt), then its the job of that commit to also change it to a recursive mutex if needed.

[iv461]

In this case currently it is not necessary, sure, and I also can't think of how you would want to call this function recursively. However, my point is that there are no significant disadvantages to adding a recursive mutex.

But by using it, you get the advantage that you cannot produce any deadlocks in the future (of which there were a couple ones here reported in rclcpp that were then resolved by using a recursive mutex).
So overall it has only advantages, no disadvatages, and it's better to be safe than sorry :)

But since you disagree, I will change it back to using a regular mutex :)