protect access to global logging calls with a mutex#562
Merged
Conversation
Signed-off-by: William Woodall <william@osrfoundation.org>
Member
Author
|
CI (full because I couldn't come up with a good way to cover what might be affected without potentially missing something):
|
ivanpauno
approved these changes
May 29, 2020
Signed-off-by: William Woodall <william@osrfoundation.org>
Member
Author
|
New CI:
|
ivanpauno
approved these changes
Jun 1, 2020
Member
Author
|
The CI failures are, in my opinion, unrelated. Just waiting on second review from @jacobperron, but I think this is gtg. |
jacobperron
approved these changes
Jun 1, 2020
sloretz
added a commit
that referenced
this pull request
Mar 11, 2021
Reverts most of #562 ros2/rclcpp#1042 describes a crash that can occur in rclcpp when rcl logging functions are called in different threads. This was fixed in ros2/rclcpp#1125, and a similar fix was made for rclpy in #562. This fix is unnecessary in rclpy because it cannot call the logging macros from multiple threads unless the GIL is released. The only place the GIL is released is around rcl_wait(), so the logging methods are already protected. Removing this code makes it a little easier to divide the remaining work of porting _rclpy.c to pybind11. If for some reason we decide to release the GIL around logging methods in the future, then they can be protected in the future using `pybind11::call_guard<T>` with a type that locks a global logging mutex when it is default constructed and unlocks it when its destructed. Signed-off-by: Shane Loretz <sloretz@openrobotics.org> Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Counter part to ros2/rclcpp#1125
Note this pull request introduces C++ code to the rclpy python library (though it keeps the main files as pure C). I started by trying to implement it without std mutex from C++, but that was taking a lot more time, so I decided to create new C functions to do the mutex locking and call those from the main file. In the future we could reimplement these few functions to be C only if we wanted to do so.