Skip to content

Conversation

@bjsowa
Copy link
Contributor

@bjsowa bjsowa commented Jun 7, 2025

Description

Fixes #1467
Fixes #1405
Fixes #1462

Is this user-facing behavior change?

  • The _pending() method in Future has been renamed to pending()
  • The user no longer has to do any workarounds with waking up the executor when a coroutine gets stuck awaiting a future that is already finished.

Did you use Generative AI?

Some of the comments and docstrings might be partially written by Github Copilot.

Additional Information

For #1467, I propagate a guard condition from a task to the awaited future and add this guard condition to the wait set in the executor. When a future is completed by an entity not controller by the executor (e.g. a separate thread), the guard condition is triggered and wakes up the executor so it can resume the task.

For #1405, I just had to make sure the task is not yielded from _wait_for_ready_callbacks when it is not ready to be resumed, hence the Task.ready() method.

EDIT: Check this comment

@bjsowa bjsowa force-pushed the fix/async-task-resume branch from 47a7eac to eeea7b0 Compare June 7, 2025 01:17
@bjsowa bjsowa changed the title Fix/async task resume Fix issues with resuming async tasks awaiting a future Jun 7, 2025
@bjsowa bjsowa force-pushed the fix/async-task-resume branch from eeea7b0 to cb2e8bb Compare June 7, 2025 13:34
@sloretz sloretz self-requested a review June 12, 2025 17:30
bjsowa added 4 commits July 9, 2025 13:10
Signed-off-by: Błażej Sowa <bsowa123@gmail.com>
Signed-off-by: Błażej Sowa <bsowa123@gmail.com>
Signed-off-by: Błażej Sowa <bsowa123@gmail.com>
Signed-off-by: Błażej Sowa <bsowa123@gmail.com>
@bjsowa bjsowa force-pushed the fix/async-task-resume branch from e5486c8 to c417824 Compare July 9, 2025 11:11
bjsowa added 7 commits July 9, 2025 13:13
Signed-off-by: Błażej Sowa <bsowa123@gmail.com>
Signed-off-by: Błażej Sowa <bsowa123@gmail.com>
Signed-off-by: Błażej Sowa <bsowa123@gmail.com>
Signed-off-by: Błażej Sowa <bsowa123@gmail.com>
Signed-off-by: Błażej Sowa <bsowa123@gmail.com>
Signed-off-by: Błażej Sowa <bsowa123@gmail.com>
Signed-off-by: Błażej Sowa <bsowa123@gmail.com>
@bjsowa bjsowa force-pushed the fix/async-task-resume branch from c417824 to 458a73c Compare July 9, 2025 11:13
Co-authored-by: Shane Loretz <shane.loretz@gmail.com>
Signed-off-by: Błażej Sowa <bsowa123@gmail.com>
@bjsowa
Copy link
Contributor Author

bjsowa commented Jul 9, 2025

After looking at #971, I revised my solution to not modify the API of Task or Future.

Changes:

  • Added a call_soon method which schedules the task to be run in the next spin and wakes up the executor.
  • The tasks schedule themselves to be run in the next spin, using the new call_soon method, if they are ready to be resumed.
  • executor._tasks list now contains only tasks that are ready to run or resumed, not all tasks.

Improvements:

Regressions:

  • When a coroutine task awaits other task, when the other task finishes, the coroutine task needs 2 spins to be resumed as the first spin will only execute a callback which schedules the coroutine task to be run in next spin. This is reflected in the test_create_task_dependent_coroutines test.
  • There are 2 tests now that result in a Segmentation fault when run with the EventsExecutor. If this solution is accepted I might look into fixing it, any help would be appreciated.

@bjsowa bjsowa requested a review from nadavelkabets July 9, 2025 11:56
@bjsowa
Copy link
Contributor Author

bjsowa commented Jul 15, 2025

@nadavelkabets Could you take a look?

@nadavelkabets
Copy link
Contributor

nadavelkabets commented Jul 15, 2025

@nadavelkabets Could you take a look?

I'll try to get to it this weekend

@JWhitleyWork
Copy link

Thank you all so much for your attention and help with implementing this change! I'll create back-port PRs for at least Jazzy and Humble unless you guys have a bot for that.

@skyegalaxy
Copy link
Member

https://github.com/Mergifyio backport kilted jazzy humble

@mergify
Copy link
Contributor

mergify bot commented Dec 3, 2025

backport kilted jazzy humble

✅ Backports have been created

Details

mergify bot pushed a commit that referenced this pull request Dec 3, 2025
Signed-off-by: Błażej Sowa <bsowa123@gmail.com>
Signed-off-by: Nadav Elkabets <elnadav12@gmail.com>
Co-authored-by: Nadav Elkabets <32939935+nadavelkabets@users.noreply.github.com>
(cherry picked from commit 9695271)

# Conflicts:
#	rclpy/rclpy/executors.py
#	rclpy/rclpy/task.py
#	rclpy/test/test_executor.py
#	rclpy/test/test_task.py
mergify bot pushed a commit that referenced this pull request Dec 3, 2025
Signed-off-by: Błażej Sowa <bsowa123@gmail.com>
Signed-off-by: Nadav Elkabets <elnadav12@gmail.com>
Co-authored-by: Nadav Elkabets <32939935+nadavelkabets@users.noreply.github.com>
(cherry picked from commit 9695271)
mergify bot pushed a commit that referenced this pull request Dec 3, 2025
Signed-off-by: Błażej Sowa <bsowa123@gmail.com>
Signed-off-by: Nadav Elkabets <elnadav12@gmail.com>
Co-authored-by: Nadav Elkabets <32939935+nadavelkabets@users.noreply.github.com>
(cherry picked from commit 9695271)

# Conflicts:
#	rclpy/rclpy/executors.py
#	rclpy/rclpy/task.py
#	rclpy/src/rclpy/events_executor/events_executor.cpp
#	rclpy/src/rclpy/events_executor/events_executor.hpp
#	rclpy/test/test_executor.py
#	rclpy/test/test_task.py
ahcorde pushed a commit that referenced this pull request Dec 8, 2025
(cherry picked from commit 9695271)

Signed-off-by: Błażej Sowa <bsowa123@gmail.com>
Signed-off-by: Nadav Elkabets <elnadav12@gmail.com>
Co-authored-by: Błażej Sowa <bsowa123@gmail.com>
Co-authored-by: Nadav Elkabets <32939935+nadavelkabets@users.noreply.github.com>
bjsowa added a commit to bjsowa/rclpy that referenced this pull request Dec 8, 2025
…#1469)

Signed-off-by: Błażej Sowa <bsowa123@gmail.com>
Signed-off-by: Nadav Elkabets <elnadav12@gmail.com>
Co-authored-by: Nadav Elkabets <32939935+nadavelkabets@users.noreply.github.com>
bjsowa added a commit to bjsowa/rclpy that referenced this pull request Jan 11, 2026
…#1469)

Signed-off-by: Błażej Sowa <bsowa123@gmail.com>
Signed-off-by: Nadav Elkabets <elnadav12@gmail.com>
Co-authored-by: Nadav Elkabets <32939935+nadavelkabets@users.noreply.github.com>
InvincibleRMC pushed a commit to InvincibleRMC/rclpy that referenced this pull request Jan 12, 2026
Signed-off-by: Błażej Sowa <bsowa123@gmail.com>
Signed-off-by: Nadav Elkabets <elnadav12@gmail.com>
Co-authored-by: Nadav Elkabets <32939935+nadavelkabets@users.noreply.github.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
bjsowa added a commit to bjsowa/rclpy that referenced this pull request Jan 12, 2026
…#1469)

Signed-off-by: Błażej Sowa <bsowa123@gmail.com>
ahcorde pushed a commit that referenced this pull request Jan 15, 2026
… (#1560)

Signed-off-by: Błażej Sowa <bsowa123@gmail.com>
Signed-off-by: Nadav Elkabets <elnadav12@gmail.com>
Co-authored-by: Nadav Elkabets <32939935+nadavelkabets@users.noreply.github.com>
Co-authored-by: Florian Vahl <git@flova.de>
bjsowa added a commit to bjsowa/rclpy that referenced this pull request Jan 19, 2026
…#1469)

Signed-off-by: Błażej Sowa <bsowa123@gmail.com>
InvincibleRMC added a commit that referenced this pull request Jan 21, 2026
)

* Fix warnings from gcc. (#1501)

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Update type_support to use new abcs

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Cleanup old test cases to use new automatic inference

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Add content-filtered-topic interfaces (#1506)

Signed-off-by: Barry Xu <Barry.Xu@sony.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Added lock to protect futures for multithreaded executor (#1477)

Signed-off-by: brennanmk <brennanmk2200@gmail.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* EventsExecutor: Handle async callbacks for services and subscriptions (#1478)

Closes #1473

Signed-off-by: Brad Martin <bmartin@fatlxception.org>
Co-authored-by: Brad Martin <bmartin@fatlxception.org>
Co-authored-by: Alejandro Hernandez Cordero <ahcorde@gmail.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* add spinning state for the Executor classes. (#1510)

Signed-off-by: Tomoya.Fujita <tomoya.fujita825@gmail.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Fixes Action.*_async futures never complete (#1308)

Per rclpy:1123 If two seperate client server actions are running in seperate executors the future given to the ActionClient will never complete due to a race condition
This fixes  the calls to rcl handles potentially leading to deadlock scenarios by adding locks to there references
Co-authored-by: Aditya Agarwal <aditya.kgp25@gmail.com>
Co-authored-by: Jonathan Blixt <jmblixt3@gmail.com>
Signed-off-by: Jonathan Blixt <jmblixt3@gmail.com>

Co-authored-by: Alejandro Hernandez Cordero <ahcorde@gmail.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* remove unused 'param_type' (#1524)

'param_type' is set but never used

Signed-off-by: Christian Rauch <Christian.Rauch@unileoben.ac.at>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Changelog

Signed-off-by: Alejandro Hernandez Cordero <ahcorde@gmail.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* 10.0.1

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Remove duplicate future handling from send_goal_async (#1532)

A recent change intended to move this logic into a lock context, but
actually ended up duplicating it instead. This fixes that by removing
the duplicated logic outside of the lock. It also preserves the explicit
typing annotation on the future.

Signed-off-by: Nathan Wiebe Neufeldt <wn.nathan@gmail.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* fix(test_events_executor): destroy all nodes before shutdown (#1538)

Signed-off-by: yuanyuyuan <az6980522@gmail.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* add BaseImpl

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Add ImplT Support

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* fix changelong

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Remove accidental tuple (#1542)

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Allow action servers without execute callback (#1219)

Signed-off-by: Tim Clephas <tim.clephas@nobleo.nl>

* add : get clients, servers info (#1307)

Signed-off-by: Minju, Lee <dlalswn531@naver.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* 10.0.2

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* update tests

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* ParameterEventHandler support ContentFiltering (#1531)

* ParameterEventHandler support ContentFiltering

Signed-off-by: Barry Xu <barry.xu@sony.com>

* Address review comments

Signed-off-by: Barry Xu <barry.xu@sony.com>

---------

Signed-off-by: Barry Xu <barry.xu@sony.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Fix issues with resuming async tasks awaiting a future (#1469)

Signed-off-by: Błażej Sowa <bsowa123@gmail.com>
Signed-off-by: Nadav Elkabets <elnadav12@gmail.com>
Co-authored-by: Nadav Elkabets <32939935+nadavelkabets@users.noreply.github.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* 10.0.3

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Increase clock accuracy (#1564)

Signed-off-by: Florian Vahl <git@flova.de>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Use unconditional wait when possible. (#1563)

Previously the max() value of the steady time was used as the default deadline. In some environments this results in overflows in the underlying pthread_cond_timedwait call, which waits for the conditional variable in the events queue implementation. Consequently, this lead to freezes in the executor. Reducing the deadline significantly helped, but using `cv.wait` instead of `cv_.wait_until` seems to be the cleaner solution.

Signed-off-by: Florian Vahl <florian.vahl@dlr.de>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Remove default from switch with enum, so that compiler warns. (#1566)

Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Fix parameter parsing for unspecified target nodes (#1552)

Signed-off-by: Barry Xu <barry.xu@sony.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Improve the compatibility of processing YAML parameter files (#1548)

Signed-off-by: Barry Xu <barry.xu@sony.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Improve wildcard parsing and optimize the logic for parsing YAML para… (#1571)

Signed-off-by: Barry Xu <barry.xu@sony.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Expose action graph functions as Node class methods. (#1574)

* Expose action graph functions as Node class methods.

Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>

* address review comments to keep the warning consistent.

Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com>

---------

Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com>

* Fix performance bug in MultiThreadedExecutor (hopefully) (#1547)

Signed-off-by: Michael Tandy <git@mjt.me.uk>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Changelog

Signed-off-by: Alejandro Hernandez Cordero <ahcorde@gmail.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* 10.0.4

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* use Msg over BaseMessage

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Use Srv over BaseService

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Use Action over BaseAction

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* lint

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

* Update rclpy/rclpy/type_support.py

Co-authored-by: Christophe Bedard <bedard.christophe@gmail.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>

---------

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
Signed-off-by: Barry Xu <Barry.Xu@sony.com>
Signed-off-by: brennanmk <brennanmk2200@gmail.com>
Signed-off-by: Brad Martin <bmartin@fatlxception.org>
Signed-off-by: Tomoya.Fujita <tomoya.fujita825@gmail.com>
Signed-off-by: Christian Rauch <Christian.Rauch@unileoben.ac.at>
Signed-off-by: Alejandro Hernandez Cordero <ahcorde@gmail.com>
Signed-off-by: Nathan Wiebe Neufeldt <wn.nathan@gmail.com>
Signed-off-by: yuanyuyuan <az6980522@gmail.com>
Signed-off-by: Tim Clephas <tim.clephas@nobleo.nl>
Signed-off-by: Minju, Lee <dlalswn531@naver.com>
Signed-off-by: Barry Xu <barry.xu@sony.com>
Signed-off-by: Błażej Sowa <bsowa123@gmail.com>
Signed-off-by: Nadav Elkabets <elnadav12@gmail.com>
Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>
Signed-off-by: Florian Vahl <git@flova.de>
Signed-off-by: Florian Vahl <florian.vahl@dlr.de>
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com>
Signed-off-by: Michael Tandy <git@mjt.me.uk>
Co-authored-by: Chris Lalancette <clalancette@gmail.com>
Co-authored-by: Barry Xu <barry.xu@sony.com>
Co-authored-by: Brennan Miller-Klugman <55165406+brennanmk@users.noreply.github.com>
Co-authored-by: Brad Martin <52003535+bmartin427@users.noreply.github.com>
Co-authored-by: Brad Martin <bmartin@fatlxception.org>
Co-authored-by: Alejandro Hernandez Cordero <ahcorde@gmail.com>
Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
Co-authored-by: Jonathan <jmblixt3@gmail.com>
Co-authored-by: Christian Rauch <Christian.Rauch@unileoben.ac.at>
Co-authored-by: Nathan Wiebe Neufeldt <wn.nathan@gmail.com>
Co-authored-by: Yuyuan Yuan <az6980522@gmail.com>
Co-authored-by: Tim Clephas <tim.clephas@nobleo.nl>
Co-authored-by: Minju, Lee <70446214+leeminju531@users.noreply.github.com>
Co-authored-by: Błażej Sowa <bsowa123@gmail.com>
Co-authored-by: Nadav Elkabets <32939935+nadavelkabets@users.noreply.github.com>
Co-authored-by: Michael Carroll <mjcarroll@intrinsic.ai>
Co-authored-by: Florian Vahl <git@flova.de>
Co-authored-by: Michael Tandy <git@mjt.me.uk>
Co-authored-by: Christophe Bedard <bedard.christophe@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

7 participants