Shutdown transition on base lifecycle node dtor may lead to segaults on subclass-registered shutdown callback about rclcpp HOT 6 CLOSED

@fujitatomoya Could you please elaborate on what drove the decision for the call to shutdown from the base class dtor? Since the callback from the base interface doesn't do anything and the base class can't call callbacks from subclasses, what were you guys trying to achieve with this change?

Subscribers to the node lifecycle state are notified of the shutdown event. Helpful for applications managing node state or for reporting to the user. That's about it, since as you point out, the subclass on_shutdown cannot be called from the base dtor.

from rclcpp.

@gabrielfpacheco thanks for creating issue. at a glance, i think we should remove all callbacks registered in the destructor since shutdown in the destructor is not user call. but there may be something more that i miss, i will try to allocate time to address this.

from rclcpp.

@fujitatomoya Could you please elaborate on what drove the decision for the call to shutdown from the base class dtor? Since the callback from the base interface doesn't do anything and the base class can't call callbacks from subclasses, what were you guys trying to achieve with this change?

from rclcpp.

Thanks for the info. Unfortunately, I think it creates more problems than it solves though.

from rclcpp.

Thanks for the info. Unfortunately, I think it creates more problems than it solves though.

Yes, it's error prone and hard to make assumptions when the register_on_shutdown method in the lifecycle class is public. Can't say I understand that design choice.

from rclcpp.

see #2520 (comment), all related to PRs are rolled back and merged.

i will go ahead to close this issue, feel free to reopen if i miss anything.

from rclcpp.