search

ros2 / launch

Tools for launching multiple processes and for writing tests involving multiple processes.
Apache License 2.0
126 stars 141 forks source link

Fix awaiting shutdown in launch context #603

Closed jacobperron closed 2 years ago

jacobperron commented 2 years ago

The method '_shutdown' is not a coroutine, and optionally returns a coroutine. Guard against the case where it returns None, otherwise we can get the following error from launch:

TypeError: object NoneType can't be used in 'await' expression

I noticed this error when a launch action raises an error during shutdown.

jacobperron commented 2 years ago

CI for packages including and above launch:

jacobperron commented 2 years ago

I don't think we can mark _shutdown as async without also changing these call sites to use await:

https://github.com/ros2/launch/blob/8adf7deccd47eb81d773acf7d2f3c285c1c04bd2/launch/launch/launch_service.py#L196-L198

https://github.com/ros2/launch/blob/8adf7deccd47eb81d773acf7d2f3c285c1c04bd2/launch/launch/launch_service.py#L418-L421

jacobperron commented 2 years ago

I don't think this PR actually works.. it sometimes results in a RuntimeWarning,

launch_service.py:349: RuntimeWarning: coroutine 'LaunchContext.emit_event' was never awaited
jacobperron commented 2 years ago

_shutdown was calling a coroutine (not awaiting it). Fixed in 7a7d556. I'm not sure how this was working before. It seems like it was implicitly being awaiting because we were calling await self._shutdown() :confused:

jacobperron commented 2 years ago

@ivanpauno this launch file reproduces the original issue:

import time
import launch

class MyActionRaisesOnShutdown(launch.action.Action):
    def __init__(self, **kwargs):
        super().__init__(**kwargs)

    def execute(self, context):
        context.add_completion_future(
            context.asyncio_loop.run_in_executor(
                None, self._foo, context
            )
        )

    def _foo(self, context):
        while True:
            if context.is_shutdown:
                raise RuntimeError('Error from MyActionRaisesOnShutdown')
            time.sleep(1.0)

def generate_launch_description():
    return launch.LaunchDescription([
        MyActionRaisesOnShutdown(),
    ])
jacobperron commented 2 years ago

This is breaking downstream tests using launch_service.shutdown; looking into it.

jacobperron commented 2 years ago