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

Remake of "fix: Fixed race condition in action server between is_ready and take" #2495

Merged
merged 2 commits into from
Apr 9, 2024

Conversation

wjwwood
Copy link
Member

@wjwwood wjwwood commented Apr 9, 2024

See: #2250

Janosch Machowinski and others added 2 commits April 4, 2024 22:47
…data and execute

Some background information: is_ready, take_data and execute data
may be called from different threads in any order. The code in the old
state expected them to be called in series, without interruption.
This lead to multiple race conditions, as the state of the pimpl objects
was altered by the three functions in a non thread safe way.

Signed-off-by: Janosch Machowinski <[email protected]>
Signed-off-by: William Woodall <[email protected]>
Signed-off-by: Janosch Machowinski <[email protected]>
Signed-off-by: William Woodall <[email protected]>
@wjwwood wjwwood self-assigned this Apr 9, 2024
@wjwwood wjwwood changed the title Remake of https:/ros2/rclcpp/pull/2250 Remake of "fix: Fixed race condition in action server between is_ready and take" Apr 9, 2024
@wjwwood
Copy link
Member Author

wjwwood commented Apr 9, 2024

New CI:

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

@wjwwood
Copy link
Member Author

wjwwood commented Apr 9, 2024

I can't approve my own pr, but lgtm. I think this will be good to merge if the CI comes back clean.

@mjcarroll
Copy link
Member

Looks like all of the generated typesupport linting failures here, I think we are good to go.

@mjcarroll mjcarroll merged commit dd81ef2 into rolling Apr 9, 2024
2 of 3 checks passed
@delete-merged-branch delete-merged-branch bot deleted the jmachowinski_2250 branch April 9, 2024 23:45
jmachowinski pushed a commit to cellumation/rclcpp that referenced this pull request May 14, 2024
…ros2#2495)

Some background information: is_ready, take_data and execute data
may be called from different threads in any order. The code in the old
state expected them to be called in series, without interruption.
This lead to multiple race conditions, as the state of the pimpl objects
was altered by the three functions in a non thread safe way.

Signed-off-by: Janosch Machowinski <[email protected]>
Signed-off-by: William Woodall <[email protected]>
Signed-off-by: Janosch Machowinski <[email protected]>
Co-authored-by: Janosch Machowinski <[email protected]>
Signed-off-by: Janosch Machowinski <[email protected]>
jmachowinski pushed a commit to cellumation/rclcpp that referenced this pull request May 14, 2024
…ros2#2495)

Some background information: is_ready, take_data and execute data
may be called from different threads in any order. The code in the old
state expected them to be called in series, without interruption.
This lead to multiple race conditions, as the state of the pimpl objects
was altered by the three functions in a non thread safe way.

Co-authored-by: William Woodall <[email protected]>
Signed-off-by: Janosch Machowinski <[email protected]>
jmachowinski pushed a commit to cellumation/rclcpp that referenced this pull request May 14, 2024
…ros2#2495)

Some background information: is_ready, take_data and execute data
may be called from different threads in any order. The code in the old
state expected them to be called in series, without interruption.
This lead to multiple race conditions, as the state of the pimpl objects
was altered by the three functions in a non thread safe way.

Co-authored-by: William Woodall <[email protected]>
Signed-off-by: Janosch Machowinski <[email protected]>
jmachowinski pushed a commit to cellumation/rclcpp that referenced this pull request Jun 25, 2024
…ros2#2495)

Some background information: is_ready, take_data and execute data
may be called from different threads in any order. The code in the old
state expected them to be called in series, without interruption.
This lead to multiple race conditions, as the state of the pimpl objects
was altered by the three functions in a non thread safe way.

Co-authored-by: William Woodall <[email protected]>
Signed-off-by: Janosch Machowinski <[email protected]>
jmachowinski pushed a commit to cellumation/rclcpp that referenced this pull request Jun 25, 2024
…ros2#2495)

Some background information: is_ready, take_data and execute data
may be called from different threads in any order. The code in the old
state expected them to be called in series, without interruption.
This lead to multiple race conditions, as the state of the pimpl objects
was altered by the three functions in a non thread safe way.

Co-authored-by: William Woodall <[email protected]>
Signed-off-by: Janosch Machowinski <[email protected]>
alsora pushed a commit that referenced this pull request Jun 27, 2024
#2531)

* fix: Fixed race condition in action server between is_ready and take" (#2495)

Some background information: is_ready, take_data and execute data
may be called from different threads in any order. The code in the old
state expected them to be called in series, without interruption.
This lead to multiple race conditions, as the state of the pimpl objects
was altered by the three functions in a non thread safe way.

Co-authored-by: William Woodall <[email protected]>
Signed-off-by: Janosch Machowinski <[email protected]>

* fix: added workaround for call to double calls to take_data

This adds a workaround for a known bug in the executor in iron.

Signed-off-by: Janosch Machowinski <[email protected]>

---------

Signed-off-by: Janosch Machowinski <[email protected]>
Co-authored-by: Janosch Machowinski <[email protected]>
Co-authored-by: William Woodall <[email protected]>
mauropasse pushed a commit to mauropasse/rclcpp that referenced this pull request Aug 9, 2024
ros2#2531)

* fix: Fixed race condition in action server between is_ready and take" (ros2#2495)

Some background information: is_ready, take_data and execute data
may be called from different threads in any order. The code in the old
state expected them to be called in series, without interruption.
This lead to multiple race conditions, as the state of the pimpl objects
was altered by the three functions in a non thread safe way.

Co-authored-by: William Woodall <[email protected]>
Signed-off-by: Janosch Machowinski <[email protected]>

* fix: added workaround for call to double calls to take_data

This adds a workaround for a known bug in the executor in iron.

Signed-off-by: Janosch Machowinski <[email protected]>

---------

Signed-off-by: Janosch Machowinski <[email protected]>
Co-authored-by: Janosch Machowinski <[email protected]>
Co-authored-by: William Woodall <[email protected]>
mauropasse pushed a commit to mauropasse/rclcpp that referenced this pull request Aug 13, 2024
ros2#2531)

* fix: Fixed race condition in action server between is_ready and take" (ros2#2495)

Some background information: is_ready, take_data and execute data
may be called from different threads in any order. The code in the old
state expected them to be called in series, without interruption.
This lead to multiple race conditions, as the state of the pimpl objects
was altered by the three functions in a non thread safe way.

Co-authored-by: William Woodall <[email protected]>
Signed-off-by: Janosch Machowinski <[email protected]>

* fix: added workaround for call to double calls to take_data

This adds a workaround for a known bug in the executor in iron.

Signed-off-by: Janosch Machowinski <[email protected]>

---------

Signed-off-by: Janosch Machowinski <[email protected]>
Co-authored-by: Janosch Machowinski <[email protected]>
Co-authored-by: William Woodall <[email protected]>
apojomovsky pushed a commit to irobot-ros/rclcpp that referenced this pull request Aug 13, 2024
* Fixes for intra-process actions (#144)

* Fixes for intra-process Actions

* Fixes for Clang builds

* Fix deadlock

* Server to store results until client requests them

* Fix feedback/result data race

See ros2#2451

* Add missing mutex

* Check return value of intra_process_action_send

---------

Co-authored-by: Mauro Passerino <[email protected]>

* Fix IPC Actions data race (#147)

* Check if goal was sent through IPC before send responses
* Add intra_process_action_server_is_available API to intra-process Client


---------

Co-authored-by: Mauro Passerino <[email protected]>

* Fix data race in Actions: Part 2 (#148)

* Fix data race in Actions: Part 2

* Fix warning - copy elision

---------

Co-authored-by: Mauro Passerino <[email protected]>

* fix: Fixed race condition in action server between is_ready and take"… (ros2#2531)

* fix: Fixed race condition in action server between is_ready and take" (ros2#2495)

Some background information: is_ready, take_data and execute data
may be called from different threads in any order. The code in the old
state expected them to be called in series, without interruption.
This lead to multiple race conditions, as the state of the pimpl objects
was altered by the three functions in a non thread safe way.

Co-authored-by: William Woodall <[email protected]>
Signed-off-by: Janosch Machowinski <[email protected]>

* fix: added workaround for call to double calls to take_data

This adds a workaround for a known bug in the executor in iron.

Signed-off-by: Janosch Machowinski <[email protected]>

---------

Signed-off-by: Janosch Machowinski <[email protected]>
Co-authored-by: Janosch Machowinski <[email protected]>
Co-authored-by: William Woodall <[email protected]>

---------

Signed-off-by: Janosch Machowinski <[email protected]>
Co-authored-by: Mauro Passerino <[email protected]>
Co-authored-by: jmachowinski <[email protected]>
Co-authored-by: Janosch Machowinski <[email protected]>
Co-authored-by: William Woodall <[email protected]>
edgarcamilocamacho added a commit to Unlimited-Robotics/third_party_rclcpp that referenced this pull request Sep 29, 2024
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.

4 participants