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.

Sign up for GitHub

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

Jump to bottom

Executor exits immediately when shut down #309

Merged
merged 2 commits into from
Apr 8, 2019
Merged

Executor exits immediately when shut down #309

merged 2 commits into from
Apr 8, 2019

Conversation

sloretz
Copy link
Contributor

@sloretz sloretz commented Apr 5, 2019

This makes executor.shutdown() trigger the executor's guard condition. Without this the executor could block forever if no more work comes in.

@sloretz
Executor exist immediately when shut down
1677a93 
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@sloretz sloretz added the in progress Actively being worked on (Kanban column) label Apr 5, 2019
@sloretz sloretz self-assigned this Apr 5, 2019
@sloretz
Spin doesn't raise ShutdownException
64e5cb5 
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@sloretz
Copy link
Contributor Author

sloretz commented Apr 5, 2019

CI (Packages above rclpy)

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

@sloretz sloretz added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Apr 5, 2019
hidmic
hidmic approved these changes Apr 8, 2019
View reviewed changes
Copy link
Contributor

@hidmic hidmic left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! All test failures seem unrelated.

@sloretz sloretz merged commit 1a3be50 into master Apr 8, 2019
@sloretz sloretz deleted the sloretz/notify_on_shutdown branch April 8, 2019 15:29
@sloretz sloretz removed the in review Waiting for review (Kanban column) label Apr 8, 2019
@sloretz sloretz mentioned this pull request Apr 8, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hidmic hidmic hidmic approved these changes

Assignees

@sloretz sloretz

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@sloretz @hidmic