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

Switch the NodeParameters lock to recursive. #781

Merged
merged 6 commits into from
Jul 11, 2019
Merged

Switch the NodeParameters lock to recursive. #781

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
55edaaf
Switch the NodeParameters lock to recursive.
clalancette Jul 9, 2019
ce8c611
Make sure that modifications can't happen within a callback.
clalancette Jul 9, 2019
646fae5
Review fixes.
clalancette Jul 10, 2019
566f519
Set parameter_modification_enabled_ in calls that make modifications.
clalancette Jul 10, 2019
a42ee00
Fix windows errors.
clalancette Jul 10, 2019
2031330
Switch to an RAII-style recursion guard.
clalancette Jul 11, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion rclcpp/include/rclcpp/node_interfaces/node_parameters.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ class NodeParameters : public NodeParametersInterface
private:
RCLCPP_DISABLE_COPY(NodeParameters)

mutable std::mutex mutex_;
mutable std::recursive_mutex mutex_;

OnParametersSetCallbackType on_parameters_set_callback_ = nullptr;

Expand Down
20 changes: 10 additions & 10 deletions rclcpp/src/rclcpp/node_interfaces/node_parameters.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ NodeParameters::declare_parameter(
const rcl_interfaces::msg::ParameterDescriptor & parameter_descriptor,
bool ignore_override)
{
std::lock_guard<std::mutex> lock(mutex_);
std::lock_guard<std::recursive_mutex> lock(mutex_);

// TODO(sloretz) parameter name validation
if (name.empty()) {
Expand Down Expand Up @@ -415,7 +415,7 @@ NodeParameters::declare_parameter(
void
NodeParameters::undeclare_parameter(const std::string & name)
{
std::lock_guard<std::mutex> lock(mutex_);
std::lock_guard<std::recursive_mutex> lock(mutex_);

auto parameter_info = parameters_.find(name);
if (parameter_info == parameters_.end()) {
Expand All @@ -434,7 +434,7 @@ NodeParameters::undeclare_parameter(const std::string & name)
bool
NodeParameters::has_parameter(const std::string & name) const
{
std::lock_guard<std::mutex> lock(mutex_);
std::lock_guard<std::recursive_mutex> lock(mutex_);

return __lockless_has_parameter(parameters_, name);
}
Expand Down Expand Up @@ -468,7 +468,7 @@ __find_parameter_by_name(
rcl_interfaces::msg::SetParametersResult
NodeParameters::set_parameters_atomically(const std::vector<rclcpp::Parameter> & parameters)
{
std::lock_guard<std::mutex> lock(mutex_);
std::lock_guard<std::recursive_mutex> lock(mutex_);

rcl_interfaces::msg::SetParametersResult result;

Expand Down Expand Up @@ -644,7 +644,7 @@ NodeParameters::set_parameters_atomically(const std::vector<rclcpp::Parameter> &
std::vector<rclcpp::Parameter>
NodeParameters::get_parameters(const std::vector<std::string> & names) const
{
std::lock_guard<std::mutex> lock(mutex_);
std::lock_guard<std::recursive_mutex> lock(mutex_);
std::vector<rclcpp::Parameter> results;
results.reserve(names.size());

Expand Down Expand Up @@ -683,7 +683,7 @@ NodeParameters::get_parameter(
const std::string & name,
rclcpp::Parameter & parameter) const
{
std::lock_guard<std::mutex> lock(mutex_);
std::lock_guard<std::recursive_mutex> lock(mutex_);

auto param_iter = parameters_.find(name);
if (
Expand All @@ -702,7 +702,7 @@ NodeParameters::get_parameters_by_prefix(
const std::string & prefix,
std::map<std::string, rclcpp::Parameter> & parameters) const
{
std::lock_guard<std::mutex> lock(mutex_);
std::lock_guard<std::recursive_mutex> lock(mutex_);

std::string prefix_with_dot = prefix.empty() ? prefix : prefix + ".";
bool ret = false;
Expand All @@ -721,7 +721,7 @@ NodeParameters::get_parameters_by_prefix(
std::vector<rcl_interfaces::msg::ParameterDescriptor>
NodeParameters::describe_parameters(const std::vector<std::string> & names) const
{
std::lock_guard<std::mutex> lock(mutex_);
std::lock_guard<std::recursive_mutex> lock(mutex_);
std::vector<rcl_interfaces::msg::ParameterDescriptor> results;
results.reserve(names.size());

Expand Down Expand Up @@ -749,7 +749,7 @@ NodeParameters::describe_parameters(const std::vector<std::string> & names) cons
std::vector<uint8_t>
NodeParameters::get_parameter_types(const std::vector<std::string> & names) const
{
std::lock_guard<std::mutex> lock(mutex_);
std::lock_guard<std::recursive_mutex> lock(mutex_);
std::vector<uint8_t> results;
results.reserve(names.size());

Expand All @@ -775,7 +775,7 @@ NodeParameters::get_parameter_types(const std::vector<std::string> & names) cons
rcl_interfaces::msg::ListParametersResult
NodeParameters::list_parameters(const std::vector<std::string> & prefixes, uint64_t depth) const
{
std::lock_guard<std::mutex> lock(mutex_);
std::lock_guard<std::recursive_mutex> lock(mutex_);
rcl_interfaces::msg::ListParametersResult result;

// TODO(mikaelarguedas) define parameter separator different from "/" to avoid ambiguity
Expand Down