fix(Executor): Fix segfault if callback group is deleted during rmw_wait#2683
Conversation
|
Not ideal, but does the job. Thanks. |
c42898b to
c58e8e7
Compare
|
We talked about a potentially better fix for this in rolling (versus Jazzy) at the client library working group, but I would be ok with a temporary fix in rolling as well. I also mentioned on the jazzy fix that it would be nice to have a regression test of some sort, but that's more important for rolling than jazzy imo. |
fujitatomoya
left a comment
There was a problem hiding this comment.
lgtm with green CI, unit test would be ideal.
| // we need to make sure that callback groups don't get out of scope | ||
| // during the wait. As in jazzy, they are not covered by the DynamicStorage, | ||
| // we explicitly hold them here as a bugfix |
There was a problem hiding this comment.
i would add TODO if we already have a plan to proper fix in the future not to miss, this does not block the PR.
c58e8e7 to
37d214f
Compare
|
Added unit test and fixed the same problem in the StaticSingleThreadedExecutor. |
Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
37d214f to
86d8313
Compare
|
Pulls: #2683
|
|
https://ci.ros2.org/job/ci_windows/22936/msbuild/new/ are unrelated. |
…ait (ros2#2683) Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com> Signed-off-by: HarunTeper <harun.teper@tu-dortmund.de>
…ait (ros2#2683) Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com> Signed-off-by: HarunTeper <harun.teper@tu-dortmund.de>
…ait (ros2#2683) Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
Fix for #2664 in rolling
Note, this is not my preferred fix for this. But I would like to merge this now,
to get a fix into jazzy ASAP.
@alsora @mjcarroll fyi