-
Notifications
You must be signed in to change notification settings - Fork 34
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
Remove stopped InputStream and StreamHandlers from Registry managers #172
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed, let's encapsulate this into PolicyManager
src/Policies.cpp
Outdated
} | ||
policy->stop(); | ||
_map.erase(name); | ||
lock.unlock(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@weyrick I've applied the same previous logic here to avoid locking HandlerManger and InputManager mutexes inside PolicyManager. here we could erase from map only at the end and keep everything in PolicyManger lock context. What do you think is safer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes let's unlock at the end - otherwise we may have a race if the policy is recreated quickly and the handler or input names collide
src/Policies.cpp
Outdated
} | ||
|
||
auto policy = _map[name].get(); | ||
std::string input_name = policy->input_stream()->name(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tiny but if you use auto
here, we'll get a const reference instead of a copy here. good habit i think.
src/Policies.cpp
Outdated
auto policy = _map[name].get(); | ||
std::string input_name = policy->input_stream()->name(); | ||
std::vector<std::string> module_names; | ||
for (auto &mod : policy->modules()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const auto&
src/Policies.cpp
Outdated
_map.erase(name); | ||
lock.unlock(); | ||
|
||
for (auto &name : module_names) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const auto&
src/Policies.cpp
Outdated
} | ||
policy->stop(); | ||
_map.erase(name); | ||
lock.unlock(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes let's unlock at the end - otherwise we may have a race if the policy is recreated quickly and the handler or input names collide
6f595b6
to
779333c
Compare
No description provided.