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

Avoid cancelling futures #616

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Avoid cancelling futures #616

wants to merge 1 commit into from

Conversation

zxzxwu
Copy link
Collaborator

@zxzxwu zxzxwu commented Dec 29, 2024

Cancelling a future doesn't provide enough information to its awaiter, and may leak asyncio.CancelledError to out of APIs.

@zxzxwu
Copy link
Collaborator Author

zxzxwu commented Dec 29, 2024

I realized that abort_on also raises asyncio.CancelledError and it's more popular...
They cannot be easily captured and overwriten inside the function, so probably we need to try and catch all code await abort_on.

Or maybe not? Since most abort_on calls are for futures instead of tasks, we can set a core.CancelledError exception for them, and let other tasks to be cancelled and captured.

@zxzxwu
Copy link
Collaborator Author

zxzxwu commented Dec 29, 2024

A truth is that asyncio.enture_future is probably always same as asyncio.create_task when given argument is a coroutine - it never works when it's not running on a event loop. So, I think maybe we can create a new future instead of the original one, and set the result or exception there like

class AbortableEventEmitter(EventEmitter):
    def abort_on(self, event: str, awaitable: Awaitable[_T]) -> asyncio.Future[_T]:
        """
        Set a coroutine or future to abort when an event occur.
        """
        real_future = asyncio.ensure_future(awaitable)
        if real_future.done():
            return real_future

        wrapped_future: asyncio.Future[_T] = asyncio.get_running_loop().create_future()

        def on_event(*_):
            if wrapped_future.done():
                return
            if isinstance(wrapped_future, asyncio.Task):
                wrapped_future.cancel()

            from bumble.core import CancelledError

            wrapped_future.set_exception(
                CancelledError(f'abort: {event} event occurred.')
            )

        def on_done(_):
            self.remove_listener(event, on_event)
            wrapped_future.set_result(real_future.result())

        self.once(event, on_event)
        real_future.add_done_callback(on_done)
        return wrapped_future

@zxzxwu zxzxwu force-pushed the cancel branch 3 times, most recently from 9479c6a to d10bd7d Compare December 30, 2024 08:18
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

Successfully merging this pull request may close these issues.

1 participant