Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RCLCPP_FATAL does not exit the program. #1869

Closed
shuhaowu opened this issue Jan 19, 2022 · 3 comments
Closed

RCLCPP_FATAL does not exit the program. #1869

shuhaowu opened this issue Jan 19, 2022 · 3 comments

Comments

@shuhaowu
Copy link

Bug report

Required Info:

  • Operating System: Ubuntu focal
  • Installation type: binaries
  • Version or commit hash: 9.2.0-1focal.20211222.233616
  • DDS implementation: N/A?
  • Client library (if applicable): rclcpp

Steps to reproduce issue

Compile and run this program:

#include "rclcpp/rclcpp.hpp"

class TestNode : public rclcpp::Node {
 public:
  TestNode() : Node("test_node") {
    RCLCPP_FATAL(get_logger(), "fatal error");
    RCLCPP_INFO(get_logger(), "shouldn't see this line");
  }
};

int main(int argc, char** argv) {
  rclcpp::init(argc, argv);
  rclcpp::spin(std::make_shared<TestNode>());
  rclcpp::shutdown();
  return 0;
}

Expected behavior

RCLCPP_FATAL should exit this program. Looking at past PR reviews, @wjwwood also expected this in this comment, if I understand that correctly.

Actual behavior

I see

[FATAL] [1642633730.613062155] [test_node]: fatal error
[INFO] [1642633730.613141427] [test_node]: shouldn't see this line
^C[INFO] [1642633740.323608940] [rclcpp]: signal_handler(signal_value=2)

Additional information

Originally discovered here: moveit/moveit2#734 (comment). In that case, a downstream consumer project (moveit2) is using RCLCPP_FATAL thinking that it will crash the program. However, it doesn't. This leads to nullptr exceptions later.

However, simply "fixing" this can have unintended side effects either. A search across Github with RCLCPP_FATAL reveals projects where they clearly know that the FATAL macro doesn't exit the program. Fixing this could break backward compatibility and be a bit problematic for them.

@clalancette
Copy link
Contributor

Looking at other logging libraries, their equivalent of FATAL doesn't crash the program either:

  • spdlog uses critical for this, and also doesn't crash.
  • log4cxx uses the name FATAL for this, and also doesn't crash
  • Python logging uses CRITICAL for this, and also doesn't crash

So I think RCLCPP_FATAL not crashing follows this pattern. @ros2/team other opinions here?

@gbiggs
Copy link
Member

gbiggs commented Jan 20, 2022

I think not crashing makes sense to me. Logging the error and how to shut down are separate concerns, and especially in robotics just dying outright is not necessarily an option all the time.

@shuhaowu
Copy link
Author

That's fine. Thanks for the clarification. I asked primarily because this PR review comment is also expecting the program to exit, which gave me some doubts...

This then would be a downstream issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants