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

Add timer to action server to check expired goals + asan fixes #343

Merged
merged 10 commits into from
Dec 1, 2018
Merged

Add timer to action server to check expired goals + asan fixes #343

merged 10 commits into from
Dec 1, 2018

Conversation

sloretz
Copy link
Contributor

@sloretz sloretz commented Nov 29, 2018 •
edited
Loading

This adds a timer to rcl_action_server_t that fires when an inactive goal handle expires and should be forgotten by the server. This tells the action server when it's time to check for expired goals.

This also includes commits from #342 only because I need both for ros2/rclcpp#593. #342 should be merged first.

I also included fixes for misc leaks/use after free/heap overflow reported by address sanitizer.

blocked by #336

@sloretz sloretz added the in review Waiting for review (Kanban column) label Nov 29, 2018
@sloretz sloretz added this to the crystal milestone Nov 29, 2018
@sloretz sloretz self-assigned this Nov 29, 2018
@sloretz
Copy link
Contributor Author

sloretz commented Nov 29, 2018

This one might conflict with #336 (will need to pass context to rcl_timer_init()) so I'll wait until that one is merged before fixing it here and running CI.

@sloretz sloretz mentioned this pull request Nov 29, 2018
Merged
@sloretz sloretz changed the title Add timer to action server to check expired goals Add timer to action server to check expired goals - also fixed issues found by address sanitizer Nov 29, 2018
@sloretz sloretz changed the title Add timer to action server to check expired goals - also fixed issues found by address sanitizer Add timer to action server to check expired goals + asan fixes Nov 30, 2018
@sloretz
Copy link
Contributor Author

sloretz commented Nov 30, 2018 •
edited
Loading

CI

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status (warnings not introduced by this PR)
  • Windows Build Status

wjwwood
wjwwood approved these changes Dec 1, 2018
View reviewed changes
Copy link
Member

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

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

I had a question or two, due to my ignorance around the actions implementation, but the changes look reasonable so far as I can tell.

rcl_action/src/rcl_action/action_server.c Show resolved Hide resolved
rcl_action/src/rcl_action/action_server.c Show resolved Hide resolved
rcl_action/src/rcl_action/action_server.c Show resolved Hide resolved
rcl_action/src/rcl_action/action_server.c Show resolved Hide resolved
rcl_action/src/rcl_action/action_server.c Show resolved Hide resolved
rcl_action/src/rcl_action/action_server.c Outdated Show resolved Hide resolved
@sloretz
Copy link
Contributor Author

sloretz commented Dec 1, 2018 •
edited
Loading

CI with 2aea0e3

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status (node died, job restarted as Build Status )
  • Windows Build Status

@sloretz sloretz merged commit fd77323 into master Dec 1, 2018
@sloretz sloretz deleted the add_server_expire_timer branch December 1, 2018 02:24
@sloretz sloretz removed the in review Waiting for review (Kanban column) label Dec 1, 2018
@sloretz sloretz mentioned this pull request Dec 7, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wjwwood wjwwood wjwwood approved these changes

@jacobperron jacobperron Awaiting requested review from jacobperron

Assignees

@sloretz sloretz

Labels
None yet
Projects
None yet
Milestone
crystal
Development

Successfully merging this pull request may close these issues.
2 participants
@sloretz @wjwwood