address context shutdown racy condition. - #3217
Conversation
Signed-off-by: Tomoya Fujita <fujita.tomoya@triorb.co.jp>
|
Tick the box to add this pull request to the merge queue (same as
|
fujitatomoya
left a comment
There was a problem hiding this comment.
this is ABI compatible, so we can backport this to downstream distros. what do you think?
|
Pulls: #3217 |
skyegalaxy
left a comment
There was a problem hiding this comment.
lgtm for fixing this in previous distros in an ABI-compatible way, although IMO, I feel like the diff in https://github.com/ros2/rclcpp/pull/3216/changes is cleaner than introducing a g_contexts_in_shutdown.
My slight preference would be for if @fishitiny 's PR had targeted rolling and included the regression test added in this PR, and this g_contexts_in_shutdown variant was only used for backports, but not a big deal if we wanna merge as-is
|
I agree this is more the back port and the other version was cleaner and should go into rolling |
|
you are right. let's use this PR only for downstream branches, and address rolling with appropriate simple one. i will create the new PR for rolling and retarget this PR to lyrical. |
Description
replaces #3216
Fixes # (issue)
Is this user-facing behavior change?
Not really, it internally addresses the racy condition on context shutdown.
Did you use Generative AI?
Yes, Claude Fable 5.
Additional Information