Remove stopped InputStream and StreamHandlers from Registry managers #172
Conversation
weyrick
left a comment
There was a problem hiding this comment.
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.
@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.
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.
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()) { |
src/Policies.cpp
Outdated
| _map.erase(name); | ||
| lock.unlock(); | ||
|
|
||
| for (auto &name : module_names) { |
src/Policies.cpp
Outdated
| } | ||
| policy->stop(); | ||
| _map.erase(name); | ||
| lock.unlock(); |
There was a problem hiding this comment.
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.