rclpy icon indicating copy to clipboard operation
rclpy copied to clipboard

Add ability to wait for MultiThreadedExecutor threads to join when shutting down

Open apockill opened this issue 2 years ago • 17 comments

Add a wait_for_threads parameter to MultiThreadedExecutor.shutdown(...) which will tell the underlying executor to shut down, and also wait for the threads to join.

Resolves https://github.com/ros2/rclpy/issues/893

I'd like some feedback on whether wait_for_threads should default to True or False.

apockill avatar Feb 19 '22 00:02 apockill

@apockill could you address DCO issue? i will review this tomorrow.

fujitatomoya avatar Feb 23 '22 02:02 fujitatomoya

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

fujitatomoya avatar Feb 23 '22 21:02 fujitatomoya

@apockill could you check on https://github.com/ros2/rclpy/pull/899/checks?check_run_id=5307997326 ?

fujitatomoya avatar Feb 23 '22 21:02 fujitatomoya

@fujitatomoya Should be good to go. Thank you!

apockill avatar Feb 23 '22 21:02 apockill

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

fujitatomoya avatar Mar 10 '22 19:03 fujitatomoya

@apockill can you rebase this fix? i believe a couple of CI failure is related to https://github.com/ros2/rclpy/commit/8c4892b3ccbbaec23f88cc4c8d10e5fc1f3ade91. I want to be sure on this, so after rebase I will restart the CI again.

fujitatomoya avatar Mar 11 '22 04:03 fujitatomoya

Okay- the PR has been rebased.

apockill avatar Mar 16 '22 21:03 apockill

@apockill we are having DCO error https://github.com/ros2/rclpy/pull/899/checks?check_run_id=5576899504, can you check?

fujitatomoya avatar Mar 16 '22 21:03 fujitatomoya

@ros-pull-request-builder retest this please

fujitatomoya avatar Mar 16 '22 21:03 fujitatomoya

It seems the DCO failures were due to my work email not being associated with github, but being signed in the git commits. I've added the email now to be associated with my github username, do you think that would fix the issue?

apockill avatar Mar 22 '22 00:03 apockill

It seems the DCO failures were due to my work email not being associated with github, but being signed in the git commits. I've added the email now to be associated with my github username, do you think that would fix the issue?

I reran the check, and it still didn't pass.

Commit sha: [392416d](https://github.com/ros2/rclpy/pull/899/commits/392416d60edea5b20aebadb069010b674a99a5a8), Author: Alex Thiel, Committer: Alex Thiel; Expected "Alex Thiel [[email protected]](mailto:[email protected])", but got "Alex Thiel [[email protected]](mailto:[email protected])".

Is the author email on the commit different from the signed off line? I'd recommend git rebase -i master and for each commit change either the author or the signed off line to match each other.

sloretz avatar Mar 25 '22 00:03 sloretz

@apockill friendly ping.

fujitatomoya avatar Mar 25 '22 15:03 fujitatomoya

@apockill unfortunately we are still having issue mentioned on https://github.com/ros2/rclpy/pull/899#issuecomment-1078521545

fujitatomoya avatar Mar 25 '22 16:03 fujitatomoya

after DCO is fixed, i will retest with CI.

fujitatomoya avatar Mar 25 '22 16:03 fujitatomoya

I'm sorry- I'm still trying to learn how to use git to fix these issues. I haven't dealt with commit rewording before :sweat_smile:

I'll get back to you soon

apockill avatar Mar 25 '22 17:03 apockill

I'm really stuck on this issue- is there any detailed documentation on how to do this process? I'm definitely running into the edge of my understanding of git.

apockill avatar Mar 25 '22 17:03 apockill

DCO error tells that https://github.com/ros2/rclpy/pull/899/commits/392416d60edea5b20aebadb069010b674a99a5a8 has been committed by Alex Thiel [email protected] but Alex Thiel [email protected]. so changing this commit with Alex Thiel [email protected] singed-off-by should fix the problem.

fujitatomoya avatar Mar 25 '22 22:03 fujitatomoya

@apockill Please run

git fetch origin rolling
git rebase origin/rolling

This will put your commits on top of trunk. Then do a git log. Count how many commits there are of yours.

Then run, replacing X with the count:

git rebase HEAD~X  --signoff
git push -f

This should sort it out.

russkel avatar Apr 19 '23 06:04 russkel

the commits from this pull request don't show up anymore. The following is the best that I can reconstruct based on the commit hashes mentioned in comments:

https://github.com/ros2/rclpy/compare/1b8cacfce7d42dd98a0191ee4b387f5323a71d15...392416d60edea5b20aebadb069010b674a99a5a8

scpeters avatar Jun 26 '24 18:06 scpeters