Shutdown action does not interrupt timers

[pbaughman]

Bug report

A launch.actions.TimerAction will keep a launch service running even after a Shutdown action

It's possible this is expected behavior, but from the description of a shutdown action I'd expect it to terminate everything right away
Required Info:

• Operating System: Ubuntu 16.04
• Installation type: source
• Version or commit hash: b665104
• DDS implementation:
• Client library (if applicable): rclpy

Steps to reproduce issue

import time                                                                 
import launch                                                               
import launch.actions                                                       
import launch_ros.actions                                                   
                                                                            
def generate_launch_description():                                          
                                                                            
    return launch.LaunchDescription([                                       
                                                                            
        launch_ros.actions.Node(                                            
            package="demo_nodes_py",                                        
            node_executable='talker',                                       
        ),                                                                  
                                                                            
        launch.actions.TimerAction(                                         
            period="5",                                                     
            actions=[                                                       
                launch.actions.Shutdown(reason="Five second timeout")       
            ]                                                               
        ),                                                                  
                                                                            
        launch.actions.TimerAction(                                         
            period="20",                                                    
            actions=[                                                       
                launch.actions.Shutdown(reason="Twenty second timeout")     
            ]                                                               
        )                                                                   
                                                                            
    ])                                                                                                                                                  
                                                                            
if __name__ == "__main__":                                                  
    launch_service = launch.LaunchService()                                 
    launch_service.include_launch_description(generate_launch_description())
    start = time.time()                                                     
    launch_service.run()                                                    
    end = time.time()                                                       
    print(end - start)                                                      

Expected behavior

I'd expect the above program to terminate after about 5 seconds, once the first TimerAction causes a shutdown

Actual behavior

The talker node is shut down after 5 seconds, but the call to launch_service.run() does not return until 20 seconds have elapsed

Additional information

Just let me know if my expectations are wrong here. Since the docs say "shut down a launched system" instead of "shut down all running processes" I sort of expect the whole thing to wrap up after 5 seconds

[wjwwood]

I didn't intentionally make it so that timers would hold up the system, however, that might be important to the design right now. For example, when you call for shutdown, timers are created, that need to not immediately expire, to escalate to SIGTERM and SIGKILL for processes that are misbehaving.

I think the appropriate solution is that timers are changed to intercept shutdown events and cancel themselves unless the creator of the timer specifies otherwise, and then for the timers that need to persist shutdown, specify that they should ignore shutdown.

For an example of how to change the timers, look at how execute process interacts with the shutdown event:

• launch/launch/launch/actions/execute_process.py

  Lines 471 to 474 in 34f10c6

  	EventHandler(
  	matcher=lambda event: is_a_subclass(event, ShutdownProcess),
  	entities=OpaqueFunction(function=self.__on_shutdown_process_event),
  	),

• launch/launch/launch/actions/execute_process.py

  Lines 240 to 244 in 34f10c6

  	def __on_shutdown_process_event(
  	self,
  	context: LaunchContext
  	) -> Optional[LaunchDescription]:
  	return self.__shutdown_process(context, send_sigint=True)

For an example of how the timer could cancel itself, look here:

• launch/launch/launch/actions/execute_process.py

  Lines 343 to 350 in 34f10c6

  	def __cleanup(self):
  	# Cancel any pending timers we started.
  	if self.__sigterm_timer is not None:
  	self.__sigterm_timer.cancel()
  	if self.__sigkill_timer is not None:
  	self.__sigkill_timer.cancel()
  	# Signal that we're done to the launch system.
  	self.__completed_future.set_result(None)

These are some of the timers that would need to be updated to ignore shutdown, but it's not an exhaustive list:

• launch/launch/launch/actions/execute_process.py

  Lines 310 to 331 in 34f10c6

  	# Setup a timer to send us a SIGTERM if we don't shutdown quickly.
  	self.__sigterm_timer = TimerAction(period=sigterm_timeout, actions=[
  	OpaqueFunction(
  	function=printer,
  	args=(base_msg.format('{}', '{}', 'SIGINT', 'SIGTERM'), sigterm_timeout)
  	),
  	EmitEvent(event=SignalProcess(
  	signal_number=signal.SIGTERM,
  	process_matcher=matches_action(self)
  	)),
  	])
  	# Setup a timer to send us a SIGKILL if we don't shutdown after SIGTERM.
  	self.__sigkill_timer = TimerAction(period=sigkill_timeout, actions=[
  	OpaqueFunction(
  	function=printer,
  	args=(base_msg.format('{}', '{}', 'SIGTERM', 'SIGKILL'), sigkill_timeout)
  	),
  	EmitEvent(event=SignalProcess(
  	signal_number='SIGKILL',
  	process_matcher=matches_action(self)
  	))
  	])

[pbaughman]

Thanks @wjwwood - I'll take a crack at this

[pbaughman]

Merged!