ros2 / ros2cli

ROS 2 command line interface tools
Apache License 2.0
173 stars 160 forks source link

Fix the SIGTERM handling in the ros2 daemon. #887

Closed clalancette closed 7 months ago

clalancette commented 7 months ago

There are 2 fixes needed here:

  1. Make sure to check rclpy.ok() in the overall loop for handling requests. That's because rclpy is handling the signals by default.
  2. Realize that server.handle_request() uses the set timeout as a timeout to select(), essentially. Because of that, we instead set the timeout to a short value (200 milliseconds), and do the overall timeout checking ourselves.

Note that because we are waking up more often, this will cause the ros2 daemon to use more CPU than before. But it should be negligible overall.

This should fix #885

clalancette commented 7 months ago

CI:

clalancette commented 7 months ago

Given that CI is clean, and I think this solves a real problem without too much downside, I'm going to go ahead and merge it in. If we find out that this causes too much problem down the line, we can always extend the timeout; it is just a tradeoff between being reactive and using CPU time.