Fix memory leak and data race

- Delete previous Layout before creating next one, and in destructor
- Use stack XKBContext instead of local new+delete
- Lock mutex in update() as it is called from a different thread than onEvent(res)
This commit is contained in:
Gavin Beatty 2021-08-20 10:06:35 -05:00 committed by GitHub
parent 8940c3bbe8
commit 9ee701974f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -94,6 +94,7 @@ void Language::onEvent(const struct Ipc::ipc_response& res) {
} }
auto Language::update() -> void { auto Language::update() -> void {
std::lock_guard<std::mutex> lock(mutex_);
auto display_layout = trim(fmt::format(format_, auto display_layout = trim(fmt::format(format_,
fmt::arg("short", layout_.short_name), fmt::arg("short", layout_.short_name),
fmt::arg("shortDescription", layout_.short_description), fmt::arg("shortDescription", layout_.short_description),
@ -125,9 +126,9 @@ auto Language::set_current_layout(std::string current_layout) -> void {
auto Language::init_layouts_map(const std::vector<std::string>& used_layouts) -> void { auto Language::init_layouts_map(const std::vector<std::string>& used_layouts) -> void {
std::map<std::string, std::vector<Layout*>> found_by_short_names; std::map<std::string, std::vector<Layout*>> found_by_short_names;
XKBContext* xkb_context_ = new XKBContext(); XKBContext xkb_context;
auto layout = xkb_context_->next_layout(); auto layout = xkb_context.next_layout();
for (; layout != nullptr; layout = xkb_context_->next_layout()) { for (; layout != nullptr; layout = xkb_context.next_layout()) {
if (std::find(used_layouts.begin(), used_layouts.end(), layout->full_name) == if (std::find(used_layouts.begin(), used_layouts.end(), layout->full_name) ==
used_layouts.end()) { used_layouts.end()) {
continue; continue;
@ -144,7 +145,6 @@ auto Language::init_layouts_map(const std::vector<std::string>& used_layouts) ->
layouts_map_.emplace(layout->full_name, *layout); layouts_map_.emplace(layout->full_name, *layout);
} }
delete xkb_context_;
if (is_variant_displayed || found_by_short_names.size() == 0) { if (is_variant_displayed || found_by_short_names.size() == 0) {
return; return;
@ -203,10 +203,13 @@ auto Language::XKBContext::next_layout() -> Layout* {
auto base_layout = base_layouts_by_name_[name]; auto base_layout = base_layouts_by_name_[name];
short_description = base_layout == nullptr ? "" : std::string(rxkb_layout_get_brief(base_layout)); short_description = base_layout == nullptr ? "" : std::string(rxkb_layout_get_brief(base_layout));
} }
delete layout_;
layout_ = new Layout{description, name, variant, short_description}; layout_ = new Layout{description, name, variant, short_description};
return layout_; return layout_;
} }
Language::XKBContext::~XKBContext() { rxkb_context_unref(context_); } Language::XKBContext::~XKBContext() {
rxkb_context_unref(context_);
delete layout_;
}
} // namespace waybar::modules::sway } // namespace waybar::modules::sway