Skip to content

Commit

Permalink
Revert "Revert "Fix potential memory leaks""
Browse files Browse the repository at this point in the history
This reverts commit 2d33c20 and
reapplies various patches for memory leaks.
The reason for the revert was a bug for a maximum duration interval
which caused sleep_for() to cause unpredictable behavior.
  • Loading branch information
taminob committed Oct 22, 2023
1 parent 5355da5 commit 552ff7e
Show file tree
Hide file tree
Showing 13 changed files with 90 additions and 29 deletions.
2 changes: 1 addition & 1 deletion include/modules/upower/upower.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class UPower : public AModule {
UpDevice *displayDevice;
guint login1_id;
GDBusConnection *login1_connection;
UPowerTooltip *upower_tooltip;
std::unique_ptr<UPowerTooltip> upower_tooltip;
std::string lastStatus;
bool showAltText;
bool upowerRunning;
Expand Down
3 changes: 2 additions & 1 deletion include/modules/upower/upower_tooltip.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include <libupower-glib/upower.h>

#include <memory>
#include <unordered_map>

#include "gtkmm/box.h"
Expand All @@ -16,7 +17,7 @@ class UPowerTooltip : public Gtk::Window {

const std::string getDeviceIcon(UpDeviceKind& kind);

Gtk::Box* contentBox;
std::unique_ptr<Gtk::Box> contentBox;

uint iconSize;
uint tooltipSpacing;
Expand Down
21 changes: 21 additions & 0 deletions include/util/scope_guard.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#pragma once

#include <utility>

namespace waybar::util {

template <typename Func>
class scope_guard {
public:
explicit scope_guard(Func&& exit_function) : f{std::forward<Func>(exit_function)} {}
scope_guard(const scope_guard&) = delete;
scope_guard(scope_guard&&) = default;
scope_guard& operator=(const scope_guard&) = delete;
scope_guard& operator=(scope_guard&&) = default;
~scope_guard() { f(); }

private:
Func f;
};

} // namespace waybar::util
2 changes: 1 addition & 1 deletion src/ALabel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ ALabel::ALabel(const Json::Value& config, const std::string& name, const std::st
: AModule(config, name, id, config["format-alt"].isString() || enable_click, enable_scroll),
format_(config_["format"].isString() ? config_["format"].asString() : format),
interval_(config_["interval"] == "once"
? std::chrono::seconds(100000000)
? std::chrono::seconds::max()
: std::chrono::seconds(
config_["interval"].isUInt() ? config_["interval"].asUInt() : interval)),
default_format_(format_) {
Expand Down
8 changes: 7 additions & 1 deletion src/modules/bluetooth.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,26 @@
#include <algorithm>
#include <sstream>

#include "util/scope_guard.hpp"

namespace {

using GDBusManager = std::unique_ptr<GDBusObjectManager, void (*)(GDBusObjectManager*)>;

auto generateManager() -> GDBusManager {
GError* error = nullptr;
waybar::util::scope_guard error_deleter([error]() {
if (error) {
g_error_free(error);
}
});
GDBusObjectManager* manager = g_dbus_object_manager_client_new_for_bus_sync(
G_BUS_TYPE_SYSTEM,
GDBusObjectManagerClientFlags::G_DBUS_OBJECT_MANAGER_CLIENT_FLAGS_DO_NOT_AUTO_START,
"org.bluez", "/", NULL, NULL, NULL, NULL, &error);

if (error) {
spdlog::error("g_dbus_object_manager_client_new_for_bus_sync() failed: {}", error->message);
g_error_free(error);
}

auto destructor = [](GDBusObjectManager* manager) {
Expand Down
7 changes: 6 additions & 1 deletion src/modules/cpu_usage/bsd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <cstdlib> // malloc

#include "modules/cpu_usage.hpp"
#include "util/scope_guard.hpp"

#if defined(__NetBSD__) || defined(__OpenBSD__)
#include <sys/sched.h>
Expand All @@ -33,6 +34,11 @@ std::vector<std::tuple<size_t, size_t>> waybar::modules::CpuUsage::parseCpuinfo(
int ncpu = sysconf(_SC_NPROCESSORS_CONF);
size_t sz = CPUSTATES * (ncpu + 1) * sizeof(pcp_time_t);
pcp_time_t *cp_time = static_cast<pcp_time_t *>(malloc(sz)), *pcp_time = cp_time;
waybar::util::scope_guard cp_time_deleter([cp_time]() {
if (cp_time) {
free(cp_time);
}
});
#if defined(__NetBSD__)
int mib[] = {
CTL_KERN,
Expand Down Expand Up @@ -97,6 +103,5 @@ std::vector<std::tuple<size_t, size_t>> waybar::modules::CpuUsage::parseCpuinfo(
}
cpuinfo.emplace_back(single_cp_time[CP_IDLE], total);
}
free(cp_time);
return cpuinfo;
}
8 changes: 7 additions & 1 deletion src/modules/custom.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

#include <spdlog/spdlog.h>

#include "util/scope_guard.hpp"

waybar::modules::Custom::Custom(const std::string& name, const std::string& id,
const Json::Value& config)
: ALabel(config, "custom-" + name, id, "{}"),
Expand Down Expand Up @@ -57,6 +59,11 @@ void waybar::modules::Custom::continuousWorker() {
}
thread_ = [this, cmd] {
char* buff = nullptr;
waybar::util::scope_guard buff_deleter([buff]() {
if (buff) {
free(buff);
}
});
size_t len = 0;
if (getline(&buff, &len, fp_) == -1) {
int exit_code = 1;
Expand Down Expand Up @@ -90,7 +97,6 @@ void waybar::modules::Custom::continuousWorker() {
output_ = {0, output};
dp.emit();
}
free(buff);
};
}

Expand Down
30 changes: 20 additions & 10 deletions src/modules/mpris/mpris.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
#include <sstream>
#include <string>

#include "util/scope_guard.hpp"

extern "C" {
#include <playerctl/playerctl.h>
}
Expand Down Expand Up @@ -117,6 +119,11 @@ Mpris::Mpris(const std::string& id, const Json::Value& config)
}

GError* error = nullptr;
waybar::util::scope_guard error_deleter([error]() {
if (error) {
g_error_free(error);
}
});
manager = playerctl_player_manager_new(&error);
if (error) {
throw std::runtime_error(fmt::format("unable to create MPRIS client: {}", error->message));
Expand All @@ -136,9 +143,7 @@ Mpris::Mpris(const std::string& id, const Json::Value& config)
} else {
GList* players = playerctl_list_players(&error);
if (error) {
auto e = fmt::format("unable to list players: {}", error->message);
g_error_free(error);
throw std::runtime_error(e);
throw std::runtime_error(fmt::format("unable to list players: {}", error->message));
}

for (auto p = players; p != NULL; p = p->next) {
Expand Down Expand Up @@ -410,8 +415,7 @@ auto Mpris::onPlayerNameAppeared(PlayerctlPlayerManager* manager, PlayerctlPlaye
return;
}

GError* error = nullptr;
mpris->player = playerctl_player_new_from_name(player_name, &error);
mpris->player = playerctl_player_new_from_name(player_name, NULL);
g_object_connect(mpris->player, "signal::play", G_CALLBACK(onPlayerPlay), mpris, "signal::pause",
G_CALLBACK(onPlayerPause), mpris, "signal::stop", G_CALLBACK(onPlayerStop),
mpris, "signal::stop", G_CALLBACK(onPlayerStop), mpris, "signal::metadata",
Expand Down Expand Up @@ -478,6 +482,11 @@ auto Mpris::getPlayerInfo() -> std::optional<PlayerInfo> {
}

GError* error = nullptr;
waybar::util::scope_guard error_deleter([error]() {
if (error) {
g_error_free(error);
}
});

char* player_status = nullptr;
auto player_playback_status = PLAYERCTL_PLAYBACK_STATUS_STOPPED;
Expand All @@ -487,9 +496,7 @@ auto Mpris::getPlayerInfo() -> std::optional<PlayerInfo> {
if (player_name == "playerctld") {
GList* players = playerctl_list_players(&error);
if (error) {
auto e = fmt::format("unable to list players: {}", error->message);
g_error_free(error);
throw std::runtime_error(e);
throw std::runtime_error(fmt::format("unable to list players: {}", error->message));
}
// > get the list of players [..] in order of activity
// https://github.com/altdesktop/playerctl/blob/b19a71cb9dba635df68d271bd2b3f6a99336a223/playerctl/playerctl-common.c#L248-L249
Expand Down Expand Up @@ -568,12 +575,16 @@ auto Mpris::getPlayerInfo() -> std::optional<PlayerInfo> {

errorexit:
spdlog::error("mpris[{}]: {}", info.name, error->message);
g_error_free(error);
return std::nullopt;
}

bool Mpris::handleToggle(GdkEventButton* const& e) {
GError* error = nullptr;
waybar::util::scope_guard error_deleter([error]() {
if (error) {
g_error_free(error);
}
});

auto info = getPlayerInfo();
if (!info) return false;
Expand Down Expand Up @@ -603,7 +614,6 @@ bool Mpris::handleToggle(GdkEventButton* const& e) {
if (error) {
spdlog::error("mpris[{}]: error running builtin on-click action: {}", (*info).name,
error->message);
g_error_free(error);
return false;
}
return true;
Expand Down
16 changes: 12 additions & 4 deletions src/modules/sni/host.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

#include <spdlog/spdlog.h>

#include "util/scope_guard.hpp"

namespace waybar::modules::SNI {

Host::Host(const std::size_t id, const Json::Value& config, const Bar& bar,
Expand Down Expand Up @@ -57,17 +59,20 @@ void Host::nameVanished(const Glib::RefPtr<Gio::DBus::Connection>& conn, const G

void Host::proxyReady(GObject* src, GAsyncResult* res, gpointer data) {
GError* error = nullptr;
waybar::util::scope_guard error_deleter([error]() {
if (error != nullptr) {
g_error_free(error);
}
});
SnWatcher* watcher = sn_watcher_proxy_new_finish(res, &error);
if (g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) {
spdlog::error("Host: {}", error->message);
g_error_free(error);
return;
}
auto host = static_cast<SNI::Host*>(data);
host->watcher_ = watcher;
if (error != nullptr) {
spdlog::error("Host: {}", error->message);
g_error_free(error);
return;
}
sn_watcher_call_register_host(host->watcher_, host->object_path_.c_str(), host->cancellable_,
Expand All @@ -76,16 +81,19 @@ void Host::proxyReady(GObject* src, GAsyncResult* res, gpointer data) {

void Host::registerHost(GObject* src, GAsyncResult* res, gpointer data) {
GError* error = nullptr;
waybar::util::scope_guard error_deleter([error]() {
if (error != nullptr) {
g_error_free(error);
}
});
sn_watcher_call_register_host_finish(SN_WATCHER(src), res, &error);
if (g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) {
spdlog::error("Host: {}", error->message);
g_error_free(error);
return;
}
auto host = static_cast<SNI::Host*>(data);
if (error != nullptr) {
spdlog::error("Host: {}", error->message);
g_error_free(error);
return;
}
g_signal_connect(host->watcher_, "item-registered", G_CALLBACK(&Host::itemRegistered), data);
Expand Down
8 changes: 7 additions & 1 deletion src/modules/sni/watcher.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

#include <spdlog/spdlog.h>

#include "util/scope_guard.hpp"

using namespace waybar::modules::SNI;

Watcher::Watcher()
Expand Down Expand Up @@ -29,14 +31,18 @@ Watcher::~Watcher() {

void Watcher::busAcquired(const Glib::RefPtr<Gio::DBus::Connection>& conn, Glib::ustring name) {
GError* error = nullptr;
waybar::util::scope_guard error_deleter([error]() {
if (error) {
g_error_free(error);
}
});
g_dbus_interface_skeleton_export(G_DBUS_INTERFACE_SKELETON(watcher_), conn->gobj(),
"/StatusNotifierWatcher", &error);
if (error != nullptr) {
// Don't print an error when a watcher is already present
if (error->code != 2) {
spdlog::error("Watcher: {}", error->message);
}
g_error_free(error);
return;
}
g_signal_connect_swapped(watcher_, "handle-register-item",
Expand Down
8 changes: 4 additions & 4 deletions src/modules/upower/upower.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ UPower::UPower(const std::string& id, const Json::Value& config)
box_.set_has_tooltip(tooltip_enabled);
if (tooltip_enabled) {
// Sets the window to use when showing the tooltip
upower_tooltip = new UPowerTooltip(iconSize, tooltip_spacing, tooltip_padding);
upower_tooltip = std::make_unique<UPowerTooltip>(iconSize, tooltip_spacing, tooltip_padding);
box_.set_tooltip_window(*upower_tooltip);
box_.signal_query_tooltip().connect(sigc::mem_fun(*this, &UPower::show_tooltip_callback));
}
Expand All @@ -72,14 +72,13 @@ UPower::UPower(const std::string& id, const Json::Value& config)
G_BUS_NAME_WATCHER_FLAGS_AUTO_START, upowerAppear,
upowerDisappear, this, NULL);

GError* error = NULL;
client = up_client_new_full(NULL, &error);
client = up_client_new_full(NULL, NULL);
if (client == NULL) {
throw std::runtime_error("Unable to create UPower client!");
}

// Connect to Login1 PrepareForSleep signal
login1_connection = g_bus_get_sync(G_BUS_TYPE_SYSTEM, NULL, &error);
login1_connection = g_bus_get_sync(G_BUS_TYPE_SYSTEM, NULL, NULL);
if (!login1_connection) {
throw std::runtime_error("Unable to connect to the SYSTEM Bus!...");
} else {
Expand All @@ -99,6 +98,7 @@ UPower::UPower(const std::string& id, const Json::Value& config)
}

UPower::~UPower() {
if (displayDevice != NULL) g_object_unref(displayDevice);
if (client != NULL) g_object_unref(client);
if (login1_id > 0) {
g_dbus_connection_signal_unsubscribe(login1_connection, login1_id);
Expand Down
3 changes: 1 addition & 2 deletions src/modules/upower/upower_tooltip.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,10 @@
namespace waybar::modules::upower {
UPowerTooltip::UPowerTooltip(uint iconSize_, uint tooltipSpacing_, uint tooltipPadding_)
: Gtk::Window(),
contentBox(std::make_unique<Gtk::Box>(Gtk::ORIENTATION_VERTICAL)),
iconSize(iconSize_),
tooltipSpacing(tooltipSpacing_),
tooltipPadding(tooltipPadding_) {
contentBox = new Gtk::Box(Gtk::ORIENTATION_VERTICAL);

// Sets the Tooltip Padding
contentBox->set_margin_top(tooltipPadding);
contentBox->set_margin_bottom(tooltipPadding);
Expand Down
3 changes: 1 addition & 2 deletions src/util/prepare_for_sleep.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ namespace {
class PrepareForSleep {
private:
PrepareForSleep() {
GError *error = NULL;
login1_connection = g_bus_get_sync(G_BUS_TYPE_SYSTEM, NULL, &error);
login1_connection = g_bus_get_sync(G_BUS_TYPE_SYSTEM, NULL, NULL);
if (!login1_connection) {
spdlog::warn("Unable to connect to the SYSTEM Bus!...");
} else {
Expand Down

0 comments on commit 552ff7e

Please sign in to comment.