Bug report

Required Info:

• Operating System:
  • All
• Installation type:
  • All
• Version or commit hash:
  • be8c5d0 (i.e. today's HEAD)
• DDS implementation:
  • All
• Client library (if applicable):
  • rclcpp

Steps to reproduce issue

This is a rough sketch of what we are trying in nav2. I doubt anything this complicated is needed. Probably just making two arbitrary callbacks and registering them in different orders will repro the issue. You just need to beat the hash algorithm basically.

class MyChild : public rclcpp::LifecycleNode {
   MyChild() {
       register_my_rcl_preshutdown_callback();
   }

  ...
};

class MyDescendant : public MyChild
{
  MyDescendant() : MyChild(blah) {

    // Super constructor already ran by this point, so the MyDescendant
    // instance should have its preshutdown callback called before my_member's
    // preshutdown callback, but we are sometimes seeing them in reverse order!
    auto my_member = std::make_shared<MyChild>(blah);

  }

  ...
};

Actual behavior

In nav2 we are working on a fix which uses the preshutdown callback mechanism. A pair of callbacks are sometimes firing in the reverse order from which they were added.

It looks like both preshutdown callbacks and on shutdown callbacks use an unordered_set:

And they are executed via an iteration of those sets, which I believe means they will execute in some arbitrary hash order?

Expected behavior

But the docs state pretty clearly that callbacks will be executed in the order they are added:

Seems like the docs are mismatched to the implementation. Unfortunately it may be annoying to reproduce since this involves a hash function... Might need to make a custom hash function to reproduce it.

Additional information

Can we please also add another line to the Context::shutdown docs regarding the preshutdown callbacks? Thanks to the people who added that callback; super handy!