From 02477810bff1defbf06f6125b364757be8ad8889 Mon Sep 17 00:00:00 2001 From: Dmitry Reznitsky Date: Sun, 8 Feb 2026 00:03:30 +0100 Subject: [PATCH 01/11] Replace duplicate format_compact lambda and ostringstream with std::format - Remove local format_compact lambda from imgui_system_panel_view.cpp and use the shared pex::format_bytes() utility instead - Replace std::ostringstream in format_bytes() with std::format for better performance in hot rendering paths Co-Authored-By: Claude Opus 4.6 --- src/core/format_utils.hpp | 13 +++++-------- src/ui/imgui/imgui_system_panel_view.cpp | 15 ++++----------- 2 files changed, 9 insertions(+), 19 deletions(-) diff --git a/src/core/format_utils.hpp b/src/core/format_utils.hpp index 5348964..c2cbcfc 100644 --- a/src/core/format_utils.hpp +++ b/src/core/format_utils.hpp @@ -1,8 +1,7 @@ #pragma once #include -#include -#include +#include #include namespace pex { @@ -26,17 +25,15 @@ inline std::string format_bytes(int64_t bytes, bool compact = true) { unit_idx++; } - std::ostringstream oss; if (unit_idx == 0) { - oss << bytes << sep << units[unit_idx]; + return std::format("{}{}{}", bytes, sep, units[unit_idx]); } else if (size >= 100.0) { - oss << std::fixed << std::setprecision(0) << size << sep << units[unit_idx]; + return std::format("{:.0f}{}{}", size, sep, units[unit_idx]); } else if (size >= 10.0) { - oss << std::fixed << std::setprecision(1) << size << sep << units[unit_idx]; + return std::format("{:.1f}{}{}", size, sep, units[unit_idx]); } else { - oss << std::fixed << std::setprecision(2) << size << sep << units[unit_idx]; + return std::format("{:.2f}{}{}", size, sep, units[unit_idx]); } - return oss.str(); } } // namespace pex diff --git a/src/ui/imgui/imgui_system_panel_view.cpp b/src/ui/imgui/imgui_system_panel_view.cpp index 26bfd00..c0bd80d 100644 --- a/src/ui/imgui/imgui_system_panel_view.cpp +++ b/src/ui/imgui/imgui_system_panel_view.cpp @@ -1,5 +1,6 @@ #include "imgui_app.hpp" #include "imgui.h" +#include "../../core/format_utils.hpp" #include #include @@ -12,14 +13,6 @@ void ImGuiApp::render_system_panel() const { return; } - // Compact bytes format like htop - auto format_compact = [](int64_t bytes) -> std::string { - if (bytes < 1024) return std::format("{}B", bytes); - if (bytes < 1024 * 1024) return std::format("{:.0f}K", bytes / 1024.0); - if (bytes < 1024LL * 1024 * 1024) return std::format("{:.1f}M", bytes / (1024.0 * 1024)); - return std::format("{:.2f}G", bytes / (1024.0 * 1024 * 1024)); - }; - const auto& mem_info_used = current_data_->memory_used; const auto& mem_info_total = current_data_->memory_total; const auto& swap_info = current_data_->swap_info; @@ -90,7 +83,7 @@ void ImGuiApp::render_system_panel() const { ImGui::SameLine(0, 0); draw_bar(mem_ratio, 120, ImVec4(0.0f, 0.6f, 0.0f, 1.0f)); ImGui::SameLine(0, 0); - ImGui::Text("] %s/%s", format_compact(mem_info_used).c_str(), format_compact(mem_info_total).c_str()); + ImGui::Text("] %s/%s", pex::format_bytes(mem_info_used).c_str(), pex::format_bytes(mem_info_total).c_str()); } { @@ -99,7 +92,7 @@ void ImGuiApp::render_system_panel() const { ImGui::SameLine(0, 0); draw_bar(swap_ratio, 120, ImVec4(0.6f, 0.0f, 0.0f, 1.0f)); ImGui::SameLine(0, 0); - ImGui::Text("] %s/%s", format_compact(swap_info.used).c_str(), format_compact(swap_info.total).c_str()); + ImGui::Text("] %s/%s", pex::format_bytes(swap_info.used).c_str(), pex::format_bytes(swap_info.total).c_str()); } ImGui::Text("Tasks: %d, %d thr; %d running", @@ -131,7 +124,7 @@ void ImGuiApp::render_system_panel() const { ImGui::SameLine(0, 0); draw_bar(mem_ratio, 80, ImVec4(0.0f, 0.6f, 0.0f, 1.0f)); ImGui::SameLine(0, 0); - ImGui::Text("]%s/%s", format_compact(mem_info_used).c_str(), format_compact(mem_info_total).c_str()); + ImGui::Text("]%s/%s", pex::format_bytes(mem_info_used).c_str(), pex::format_bytes(mem_info_total).c_str()); ImGui::SameLine(); ImGui::Text("Tasks:%d Load:%.1f", current_data_->process_count, load.one_min); } From 348937746997f6a6c4e5f4dead4468da7e8420d1 Mon Sep 17 00:00:00 2001 From: Dmitry Reznitsky Date: Sun, 8 Feb 2026 00:05:00 +0100 Subject: [PATCH 02/11] Validate socket path length before using XDG_RUNTIME_DIR The sun_path field in sockaddr_un has a platform-specific size limit (~104-108 bytes). If XDG_RUNTIME_DIR produces a path exceeding this limit, fall back to the /tmp-based path which always fits. Co-Authored-By: Claude Opus 4.6 --- src/core/services/single_instance.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/core/services/single_instance.cpp b/src/core/services/single_instance.cpp index c93941c..c194f0d 100644 --- a/src/core/services/single_instance.cpp +++ b/src/core/services/single_instance.cpp @@ -30,10 +30,16 @@ SingleInstance::~SingleInstance() { } std::string SingleInstance::get_socket_path() { + constexpr size_t max_path = sizeof(sockaddr_un{}.sun_path) - 1; + if (const char* runtime_dir = std::getenv("XDG_RUNTIME_DIR")) { - return std::string(runtime_dir) + "/pex.sock"; + std::string path = std::string(runtime_dir) + "/pex.sock"; + if (path.size() <= max_path) { + return path; + } + // XDG_RUNTIME_DIR path too long, fall through to /tmp } - // Fallback: use /tmp with UID + // Fallback: use /tmp with UID (always fits) return "/tmp/pex-" + std::to_string(getuid()) + ".sock"; } From 785cd0a2088ec9ff106b23f522991edb371ff891 Mon Sep 17 00:00:00 2001 From: Dmitry Reznitsky Date: Sun, 8 Feb 2026 00:06:43 +0100 Subject: [PATCH 03/11] Narrow catch(...) to catch(const std::exception&) in platform code Prevents silently swallowing non-standard exceptions like std::bad_alloc while still handling expected filesystem_error, invalid_argument, and out_of_range exceptions from /proc iteration and string parsing. Co-Authored-By: Claude Opus 4.6 --- src/platform/linux/linux_process_killer.cpp | 4 ++-- src/platform/linux/procfs/procfs_fds.cpp | 4 ++-- src/platform/linux/procfs/procfs_network.cpp | 4 ++-- src/platform/linux/procfs/procfs_processes.cpp | 2 +- src/platform/linux/procfs/procfs_threads.cpp | 4 ++-- src/platform/solaris/solaris_process_data_provider.cpp | 4 ++-- src/platform/solaris/solaris_process_details.cpp | 10 +++++----- src/platform/solaris/solaris_process_killer.cpp | 4 ++-- 8 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/platform/linux/linux_process_killer.cpp b/src/platform/linux/linux_process_killer.cpp index 9aa6edd..cc42490 100644 --- a/src/platform/linux/linux_process_killer.cpp +++ b/src/platform/linux/linux_process_killer.cpp @@ -84,12 +84,12 @@ void LinuxProcessKiller::collect_descendants_from_proc(const int root_pid, if (meta.ppid > 0) { children_map[meta.ppid].push_back(pid); } - } catch (...) { + } catch (const std::exception&) { // Process disappeared, skip it continue; } } - } catch (...) { + } catch (const std::exception&) { // Directory iteration failed return; } diff --git a/src/platform/linux/procfs/procfs_fds.cpp b/src/platform/linux/procfs/procfs_fds.cpp index a3f9b47..b1d62a1 100644 --- a/src/platform/linux/procfs/procfs_fds.cpp +++ b/src/platform/linux/procfs/procfs_fds.cpp @@ -46,11 +46,11 @@ std::vector ProcfsReader::get_file_handles(const int pid) { } handles.push_back(std::move(handle)); - } catch (...) { + } catch (const std::exception&) { continue; } } - } catch (...) { + } catch (const std::exception&) { } std::ranges::sort(handles, [](const auto& a, const auto& b) { diff --git a/src/platform/linux/procfs/procfs_network.cpp b/src/platform/linux/procfs/procfs_network.cpp index 714bb37..ad6efa6 100644 --- a/src/platform/linux/procfs/procfs_network.cpp +++ b/src/platform/linux/procfs/procfs_network.cpp @@ -116,11 +116,11 @@ std::vector ProcfsReader::get_network_connections(const i std::from_chars(start, end, inode); if (inode > 0) socket_inodes.insert(inode); } - } catch (...) { + } catch (const std::exception&) { continue; } } - } catch (...) { + } catch (const std::exception&) { return result; } diff --git a/src/platform/linux/procfs/procfs_processes.cpp b/src/platform/linux/procfs/procfs_processes.cpp index a0583ad..7583081 100644 --- a/src/platform/linux/procfs/procfs_processes.cpp +++ b/src/platform/linux/procfs/procfs_processes.cpp @@ -34,7 +34,7 @@ std::vector ProcfsReader::get_all_processes(const int64_t total_mem if (auto info = get_process_info(pid, total_memory)) { processes.push_back(std::move(*info)); } - } catch (...) { + } catch (const std::exception&) { continue; } } diff --git a/src/platform/linux/procfs/procfs_threads.cpp b/src/platform/linux/procfs/procfs_threads.cpp index fcd1695..f7d027b 100644 --- a/src/platform/linux/procfs/procfs_threads.cpp +++ b/src/platform/linux/procfs/procfs_threads.cpp @@ -145,11 +145,11 @@ std::vector ProcfsReader::get_threads(int pid) { } threads.push_back(std::move(thread)); - } catch (...) { + } catch (const std::exception&) { continue; } } - } catch (...) { + } catch (const std::exception&) { } return threads; diff --git a/src/platform/solaris/solaris_process_data_provider.cpp b/src/platform/solaris/solaris_process_data_provider.cpp index 1e0b175..0f146a2 100644 --- a/src/platform/solaris/solaris_process_data_provider.cpp +++ b/src/platform/solaris/solaris_process_data_provider.cpp @@ -122,7 +122,7 @@ std::optional SolarisProcessDataProvider::read_process_info(int pid std::string exe_path = proc_path + "/path/a.out"; try { info.executable_path = fs::read_symlink(exe_path).string(); - } catch (...) { + } catch (const std::exception&) { info.executable_path = info.name; } @@ -145,7 +145,7 @@ std::vector SolarisProcessDataProvider::get_all_processes(int64_t t int pid = 0; try { pid = std::stoi(name); - } catch (...) { + } catch (const std::exception&) { continue; // Not a PID directory } diff --git a/src/platform/solaris/solaris_process_details.cpp b/src/platform/solaris/solaris_process_details.cpp index 5bd8730..9ae5a64 100644 --- a/src/platform/solaris/solaris_process_details.cpp +++ b/src/platform/solaris/solaris_process_details.cpp @@ -291,7 +291,7 @@ std::vector SolarisProcessDataProvider::get_threads(int pid) { int lwpid = 0; try { lwpid = std::stoi(lwpid_str); - } catch (...) { + } catch (const std::exception&) { continue; } @@ -343,7 +343,7 @@ std::vector SolarisProcessDataProvider::get_file_handles(int pid int fd_num = 0; try { fd_num = std::stoi(fd_str); - } catch (...) { + } catch (const std::exception&) { continue; } @@ -355,12 +355,12 @@ std::vector SolarisProcessDataProvider::get_file_handles(int pid try { std::string target = fs::read_symlink(link_path).string(); fh.path = target; - } catch (...) { + } catch (const std::exception&) { // Fallback: try /proc//fd/ try { std::string target = fs::read_symlink(entry.path()).string(); fh.path = target; - } catch (...) { + } catch (const std::exception&) { fh.path = "[unknown]"; } } @@ -453,7 +453,7 @@ std::vector SolarisProcessDataProvider::get_network_conne try { int fd_num = 0; fd_num = std::stoi(fd_str); - } catch (...) { + } catch (const std::exception&) { continue; } diff --git a/src/platform/solaris/solaris_process_killer.cpp b/src/platform/solaris/solaris_process_killer.cpp index 105120b..497dc66 100644 --- a/src/platform/solaris/solaris_process_killer.cpp +++ b/src/platform/solaris/solaris_process_killer.cpp @@ -80,7 +80,7 @@ KillResult SolarisProcessKiller::kill_process_tree(int pid, bool force) { int proc_pid = 0; try { proc_pid = std::stoi(name); - } catch (...) { + } catch (const std::exception&) { continue; } @@ -97,7 +97,7 @@ KillResult SolarisProcessKiller::kill_process_tree(int pid, bool force) { all_procs.emplace_back(proc_pid, psinfo.pr_ppid); } } - } catch (...) { + } catch (const std::exception&) { // Directory iteration failed } From 0e8f1d483c8fcf61eadb57bbd0c2c65149900c55 Mon Sep 17 00:00:00 2001 From: Dmitry Reznitsky Date: Sun, 8 Feb 2026 00:08:30 +0100 Subject: [PATCH 04/11] Use unordered_map for PID-keyed maps in DataStore and UI Replace std::map with std::unordered_map for process_map, previous_cpu_times_, and tree-building temporaries. These maps are keyed by int PIDs and only need point lookups, making O(1) hash lookups faster than O(log n) tree lookups on the hot path. Co-Authored-By: Claude Opus 4.6 --- src/core/services/data_store.cpp | 11 ++++++----- src/core/services/data_store.hpp | 8 ++++---- src/ui/tui/tui_process_list.cpp | 10 +++++----- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/core/services/data_store.cpp b/src/core/services/data_store.cpp index fcbb288..095a952 100644 --- a/src/core/services/data_store.cpp +++ b/src/core/services/data_store.cpp @@ -2,6 +2,7 @@ #include #include #include +#include namespace pex { @@ -166,7 +167,7 @@ void DataStore::collect_data() { }); // Build process tree - std::map> nodes; + std::unordered_map> nodes; for (auto& proc : processes) { auto node = std::make_unique(); node->info = std::move(proc); @@ -183,7 +184,7 @@ void DataStore::collect_data() { } // Build children map - std::map> children_map; + std::unordered_map> children_map; for (auto& [pid, node] : nodes) { if (int ppid = node->info.parent_pid; ppid != pid && nodes.contains(ppid)) { children_map[ppid].push_back(pid); @@ -191,8 +192,8 @@ void DataStore::collect_data() { } // Recursive function to attach children - std::function>&)> attach_children; - attach_children = [&](ProcessNode* parent, std::map>& all_nodes) { + std::function>&)> attach_children; + attach_children = [&](ProcessNode* parent, std::unordered_map>& all_nodes) { if (const auto it = children_map.find(parent->info.pid); it != children_map.end()) { for (int child_pid : it->second) { if (auto child_it = all_nodes.find(child_pid); child_it != all_nodes.end()) { @@ -324,7 +325,7 @@ void DataStore::calculate_tree_totals(ProcessNode& node) { } } -void DataStore::build_process_map(ProcessNode* node, std::map& map) { +void DataStore::build_process_map(ProcessNode* node, std::unordered_map& map) { map[node->info.pid] = node; for (auto& child : node->children) { build_process_map(child.get(), map); diff --git a/src/core/services/data_store.hpp b/src/core/services/data_store.hpp index f4ffd00..2bea79b 100644 --- a/src/core/services/data_store.hpp +++ b/src/core/services/data_store.hpp @@ -6,7 +6,7 @@ #include "../model/errors.hpp" #include "../model/system_info.hpp" #include -#include +#include #include #include #include @@ -38,7 +38,7 @@ struct ProcessNode { // are shared via shared_ptr (never modified after construction). struct DataSnapshot { std::vector> process_tree; - std::map process_map; // Non-owning pointers into process_tree + std::unordered_map process_map; // Non-owning pointers into process_tree // System stats int process_count = 0; @@ -104,7 +104,7 @@ class DataStore { void collection_thread_func(); void collect_data(); static void calculate_tree_totals(ProcessNode& node); - static void build_process_map(ProcessNode* node, std::map& map); + static void build_process_map(ProcessNode* node, std::unordered_map& map); // Injected providers (owned externally) IProcessDataProvider* process_provider_; @@ -131,7 +131,7 @@ class DataStore { std::vector per_cpu_usage_buffer_; // Reused buffer std::vector per_cpu_user_buffer_; // Reused buffer std::vector per_cpu_system_buffer_; // Reused buffer - std::map> previous_cpu_times_; + std::unordered_map> previous_cpu_times_; // Callback std::function on_data_updated_; diff --git a/src/ui/tui/tui_process_list.cpp b/src/ui/tui/tui_process_list.cpp index be3e1ae..173131c 100644 --- a/src/ui/tui/tui_process_list.cpp +++ b/src/ui/tui/tui_process_list.cpp @@ -3,13 +3,13 @@ #include #include #include -#include +#include #include namespace pex { // Helper to check if a node is the last child of its parent -static bool is_last_child(ProcessNode* node, const std::map& process_map) { +static bool is_last_child(ProcessNode* node, const std::unordered_map& process_map) { if (!process_map.contains(node->info.parent_pid)) return true; const ProcessNode* parent = process_map.at(node->info.parent_pid); @@ -19,12 +19,12 @@ static bool is_last_child(ProcessNode* node, const std::map& } // Check if node has a visible parent in the tree -static bool has_visible_parent(ProcessNode* node, const std::map& process_map) { +static bool has_visible_parent(ProcessNode* node, const std::unordered_map& process_map) { return process_map.contains(node->info.parent_pid); } // Build list of ancestors from node to root (excluding node itself) -static std::vector get_ancestors(ProcessNode* node, const std::map& process_map) { +static std::vector get_ancestors(ProcessNode* node, const std::unordered_map& process_map) { std::vector ancestors; int pid = node->info.parent_pid; int prev_pid = node->info.pid; @@ -48,7 +48,7 @@ static std::vector get_ancestors(ProcessNode* node, const std::map // Build the tree connector prefix for a node // Returns a vector where each entry indicates whether to draw │ (true) or space (false) -static std::vector get_tree_path(ProcessNode* node, const std::map& process_map) { +static std::vector get_tree_path(ProcessNode* node, const std::unordered_map& process_map) { std::vector path; // Get all ancestors (oldest first) From 99c737eb21b4abba6b184b47dae67567b3fe8853 Mon Sep 17 00:00:00 2001 From: Dmitry Reznitsky Date: Sun, 8 Feb 2026 00:09:04 +0100 Subject: [PATCH 05/11] Use binary search for thread library detection in address map Replace linear scan with std::upper_bound for O(log n) lookups when mapping thread instruction pointers to library names. The address_map from /proc/pid/maps is already sorted by start address. Co-Authored-By: Claude Opus 4.6 --- src/platform/linux/procfs/procfs_threads.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/platform/linux/procfs/procfs_threads.cpp b/src/platform/linux/procfs/procfs_threads.cpp index f7d027b..6e5d6ce 100644 --- a/src/platform/linux/procfs/procfs_threads.cpp +++ b/src/platform/linux/procfs/procfs_threads.cpp @@ -57,9 +57,13 @@ std::vector ProcfsReader::get_threads(int pid) { } auto find_library = [&](const uint64_t addr) -> std::string { - for (const auto& range : address_map) { - if (addr >= range.start && addr < range.end) { - return range.library; + // address_map is sorted by start address (from /proc/pid/maps) + auto it = std::upper_bound(address_map.begin(), address_map.end(), addr, + [](uint64_t a, const AddressRange& r) { return a < r.start; }); + if (it != address_map.begin()) { + --it; + if (addr >= it->start && addr < it->end) { + return it->library; } } return {}; From 2b03f71227f85b14faaf8f3a8f6d56e65c6918c4 Mon Sep 17 00:00:00 2001 From: Dmitry Reznitsky Date: Sun, 8 Feb 2026 00:11:00 +0100 Subject: [PATCH 06/11] Cache kstat handle in Solaris system data provider Replace per-call kstat_open()/kstat_close() with a cached handle that persists across calls. Uses kstat_chain_update() to keep the chain current and reopens on failure. Eliminates ~5 kstat_open() calls per data collection cycle. Co-Authored-By: Claude Opus 4.6 --- .../solaris/solaris_system_data_provider.cpp | 70 ++++++++++++------- .../solaris/solaris_system_data_provider.hpp | 12 ++++ 2 files changed, 56 insertions(+), 26 deletions(-) diff --git a/src/platform/solaris/solaris_system_data_provider.cpp b/src/platform/solaris/solaris_system_data_provider.cpp index 96661b5..9046d96 100644 --- a/src/platform/solaris/solaris_system_data_provider.cpp +++ b/src/platform/solaris/solaris_system_data_provider.cpp @@ -16,17 +16,41 @@ namespace pex { +SolarisSystemDataProvider::SolarisSystemDataProvider() { + kc_ = kstat_open(); +} + +SolarisSystemDataProvider::~SolarisSystemDataProvider() { + if (kc_) { + kstat_close(kc_); + kc_ = nullptr; + } +} + +void SolarisSystemDataProvider::ensure_kstat() const { + if (!kc_) { + kc_ = kstat_open(); + return; + } + // Update the kstat chain to pick up new/removed kstats + if (kstat_chain_update(kc_) == -1) { + // Chain update failed, reopen + kstat_close(kc_); + kc_ = kstat_open(); + } +} + CpuTimes SolarisSystemDataProvider::get_cpu_times() { CpuTimes times; - kstat_ctl_t* kc = kstat_open(); - if (!kc) return times; + ensure_kstat(); + if (!kc_) return times; // Aggregate CPU times from all cpu_stat instances - for (kstat_t* ksp = kc->kc_chain; ksp != nullptr; ksp = ksp->ks_next) { + for (kstat_t* ksp = kc_->kc_chain; ksp != nullptr; ksp = ksp->ks_next) { if (strcmp(ksp->ks_module, "cpu_stat") != 0) continue; - if (kstat_read(kc, ksp, nullptr) < 0) continue; + if (kstat_read(kc_, ksp, nullptr) < 0) continue; cpu_stat_t* cs = reinterpret_cast(ksp->ks_data); if (!cs) continue; @@ -38,7 +62,6 @@ CpuTimes SolarisSystemDataProvider::get_cpu_times() { // Solaris doesn't have nice, irq, softirq, steal in cpu_stat } - kstat_close(kc); return times; } @@ -53,16 +76,16 @@ void SolarisSystemDataProvider::get_per_cpu_times(std::vector& out) { out.clear(); out.resize(ncpu); - kstat_ctl_t* kc = kstat_open(); - if (!kc) return; + ensure_kstat(); + if (!kc_) return; - for (kstat_t* ksp = kc->kc_chain; ksp != nullptr; ksp = ksp->ks_next) { + for (kstat_t* ksp = kc_->kc_chain; ksp != nullptr; ksp = ksp->ks_next) { if (strcmp(ksp->ks_module, "cpu_stat") != 0) continue; int cpu_id = ksp->ks_instance; if (cpu_id < 0 || cpu_id >= ncpu) continue; - if (kstat_read(kc, ksp, nullptr) < 0) continue; + if (kstat_read(kc_, ksp, nullptr) < 0) continue; cpu_stat_t* cs = reinterpret_cast(ksp->ks_data); if (!cs) continue; @@ -72,8 +95,6 @@ void SolarisSystemDataProvider::get_per_cpu_times(std::vector& out) { out[cpu_id].idle = cs->cpu_sysinfo.cpu[CPU_IDLE]; out[cpu_id].iowait = cs->cpu_sysinfo.cpu[CPU_WAIT]; } - - kstat_close(kc); } MemoryInfo SolarisSystemDataProvider::get_memory_info() { @@ -86,10 +107,10 @@ MemoryInfo SolarisSystemDataProvider::get_memory_info() { // Available memory - use kstat for freemem // Note: kstat API uses char* not const char*, but doesn't modify the strings - kstat_ctl_t* kc = kstat_open(); - if (kc) { - kstat_t* ksp = kstat_lookup(kc, const_cast("unix"), 0, const_cast("system_pages")); - if (ksp && kstat_read(kc, ksp, nullptr) >= 0) { + ensure_kstat(); + if (kc_) { + kstat_t* ksp = kstat_lookup(kc_, const_cast("unix"), 0, const_cast("system_pages")); + if (ksp && kstat_read(kc_, ksp, nullptr) >= 0) { kstat_named_t* kn; // Free memory @@ -100,7 +121,6 @@ MemoryInfo SolarisSystemDataProvider::get_memory_info() { // Could also check availrmem, lotsfree, etc. for better estimation } - kstat_close(kc); } info.used = info.total - info.available; @@ -189,10 +209,10 @@ UptimeInfo SolarisSystemDataProvider::get_uptime() { UptimeInfo info; // Use kstat to get boot time - kstat_ctl_t* kc = kstat_open(); - if (kc) { - kstat_t* ksp = kstat_lookup(kc, const_cast("unix"), 0, const_cast("system_misc")); - if (ksp && kstat_read(kc, ksp, nullptr) >= 0) { + ensure_kstat(); + if (kc_) { + kstat_t* ksp = kstat_lookup(kc_, const_cast("unix"), 0, const_cast("system_misc")); + if (ksp && kstat_read(kc_, ksp, nullptr) >= 0) { kstat_named_t* kn = reinterpret_cast( kstat_data_lookup(ksp, const_cast("boot_time"))); if (kn) { @@ -201,7 +221,6 @@ UptimeInfo SolarisSystemDataProvider::get_uptime() { info.uptime_seconds = now - boot_time; } } - kstat_close(kc); } // Idle time would need to be calculated from CPU idle percentage @@ -221,17 +240,16 @@ long SolarisSystemDataProvider::get_clock_ticks_per_second() const { uint64_t SolarisSystemDataProvider::get_boot_time_ticks() const { uint64_t boot_time = 0; - kstat_ctl_t* kc = kstat_open(); - if (kc) { - kstat_t* ksp = kstat_lookup(kc, const_cast("unix"), 0, const_cast("system_misc")); - if (ksp && kstat_read(kc, ksp, nullptr) >= 0) { + ensure_kstat(); + if (kc_) { + kstat_t* ksp = kstat_lookup(kc_, const_cast("unix"), 0, const_cast("system_misc")); + if (ksp && kstat_read(kc_, ksp, nullptr) >= 0) { kstat_named_t* kn = reinterpret_cast( kstat_data_lookup(ksp, const_cast("boot_time"))); if (kn) { boot_time = kn->value.ul * sysconf(_SC_CLK_TCK); } } - kstat_close(kc); } return boot_time; diff --git a/src/platform/solaris/solaris_system_data_provider.hpp b/src/platform/solaris/solaris_system_data_provider.hpp index 8af5443..742540b 100644 --- a/src/platform/solaris/solaris_system_data_provider.hpp +++ b/src/platform/solaris/solaris_system_data_provider.hpp @@ -1,11 +1,19 @@ #pragma once #include "../interfaces/i_system_data_provider.hpp" +#include namespace pex { class SolarisSystemDataProvider : public ISystemDataProvider { public: + SolarisSystemDataProvider(); + ~SolarisSystemDataProvider(); + + // Non-copyable due to kstat handle ownership + SolarisSystemDataProvider(const SolarisSystemDataProvider&) = delete; + SolarisSystemDataProvider& operator=(const SolarisSystemDataProvider&) = delete; + CpuTimes get_cpu_times() override; std::vector get_per_cpu_times() override; void get_per_cpu_times(std::vector& out) override; @@ -17,6 +25,10 @@ class SolarisSystemDataProvider : public ISystemDataProvider { long get_clock_ticks_per_second() const override; uint64_t get_boot_time_ticks() const override; [[nodiscard]] std::string get_system_info_string() const override; + +private: + mutable kstat_ctl_t* kc_ = nullptr; + void ensure_kstat() const; }; } // namespace pex From 41c545639c2064505ea2f103f237136089bee97f Mon Sep 17 00:00:00 2001 From: Dmitry Reznitsky Date: Sun, 8 Feb 2026 00:12:19 +0100 Subject: [PATCH 07/11] Add PID-reuse verification to FreeBSD and Solaris process tree kill Before sending signals during tree kill, re-verify that each PID still refers to the original process by comparing start times. This prevents accidentally killing an unrelated process if the PID was recycled between collection and signal delivery, matching the existing Linux implementation's safety check. Co-Authored-By: Claude Opus 4.6 --- .../freebsd/freebsd_process_killer.cpp | 31 ++++++++++++++++ .../solaris/solaris_process_killer.cpp | 36 +++++++++++++++++-- 2 files changed, 64 insertions(+), 3 deletions(-) diff --git a/src/platform/freebsd/freebsd_process_killer.cpp b/src/platform/freebsd/freebsd_process_killer.cpp index 37381f1..54898b9 100644 --- a/src/platform/freebsd/freebsd_process_killer.cpp +++ b/src/platform/freebsd/freebsd_process_killer.cpp @@ -8,11 +8,28 @@ #include #include #include +#include #include #include namespace pex { +namespace { + +// Verify a PID still refers to the same process by comparing start times +bool is_same_process(int pid, const struct timeval& expected_start) { + int mib[4] = { CTL_KERN, KERN_PROC, KERN_PROC_PID, pid }; + struct kinfo_proc kp; + size_t len = sizeof(kp); + if (sysctl(mib, 4, &kp, &len, nullptr, 0) < 0 || len != sizeof(kp)) { + return false; + } + return kp.ki_start.tv_sec == expected_start.tv_sec && + kp.ki_start.tv_usec == expected_start.tv_usec; +} + +} // anonymous namespace + KillResult FreeBSDProcessKiller::kill_process(int pid, bool force) { KillResult result; if (pid <= 0) { @@ -90,6 +107,12 @@ KillResult FreeBSDProcessKiller::kill_process_tree(int pid, bool force) { size_t count = len / sizeof(struct kinfo_proc); struct kinfo_proc* kp = reinterpret_cast(buf.data()); + // Capture start times for PID-reuse verification + std::unordered_map start_times; + for (size_t i = 0; i < count; ++i) { + start_times[kp[i].ki_pid] = kp[i].ki_start; + } + // Find all children iteratively bool found_new = true; while (found_new) { @@ -106,10 +129,18 @@ KillResult FreeBSDProcessKiller::kill_process_tree(int pid, bool force) { int sig = force ? SIGKILL : SIGTERM; bool any_success = false; bool any_permission_denied = false; + int skipped = 0; // Kill children first (reverse order to kill deepest first) std::vector sorted_pids(pids_to_kill.begin(), pids_to_kill.end()); for (auto it = sorted_pids.rbegin(); it != sorted_pids.rend(); ++it) { + // Verify PID hasn't been reused before killing + if (auto st = start_times.find(*it); st != start_times.end()) { + if (!is_same_process(*it, st->second)) { + skipped++; + continue; + } + } if (::kill(*it, sig) == 0) { any_success = true; } else if (errno == EPERM) { diff --git a/src/platform/solaris/solaris_process_killer.cpp b/src/platform/solaris/solaris_process_killer.cpp index 497dc66..fd312cd 100644 --- a/src/platform/solaris/solaris_process_killer.cpp +++ b/src/platform/solaris/solaris_process_killer.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -18,6 +19,25 @@ namespace fs = std::filesystem; namespace pex { +namespace { + +// Verify a PID still refers to the same process by comparing start times +bool is_same_process(int pid, const timestruc_t& expected_start) { + std::string path = "/proc/" + std::to_string(pid) + "/psinfo"; + int fd = open(path.c_str(), O_RDONLY); + if (fd < 0) return false; + + psinfo_t psinfo; + ssize_t n = read(fd, &psinfo, sizeof(psinfo)); + close(fd); + + if (n != static_cast(sizeof(psinfo))) return false; + return psinfo.pr_start.tv_sec == expected_start.tv_sec && + psinfo.pr_start.tv_nsec == expected_start.tv_nsec; +} + +} // anonymous namespace + KillResult SolarisProcessKiller::kill_process(int pid, bool force) { KillResult result; if (pid <= 0) { @@ -69,8 +89,9 @@ KillResult SolarisProcessKiller::kill_process_tree(int pid, bool force) { std::set pids_to_kill; pids_to_kill.insert(pid); - // Build map of pid -> ppid + // Build map of pid -> ppid, capturing start times for PID-reuse verification std::vector> all_procs; // (pid, ppid) + std::unordered_map start_times; try { for (const auto& entry : fs::directory_iterator("/proc")) { @@ -84,7 +105,7 @@ KillResult SolarisProcessKiller::kill_process_tree(int pid, bool force) { continue; } - // Read psinfo to get parent PID + // Read psinfo to get parent PID and start time std::string psinfo_path = entry.path().string() + "/psinfo"; int fd = open(psinfo_path.c_str(), O_RDONLY); if (fd < 0) continue; @@ -93,8 +114,9 @@ KillResult SolarisProcessKiller::kill_process_tree(int pid, bool force) { ssize_t n = read(fd, &psinfo, sizeof(psinfo)); close(fd); - if (n == sizeof(psinfo)) { + if (n == static_cast(sizeof(psinfo))) { all_procs.emplace_back(proc_pid, psinfo.pr_ppid); + start_times[proc_pid] = psinfo.pr_start; } } } catch (const std::exception&) { @@ -117,10 +139,18 @@ KillResult SolarisProcessKiller::kill_process_tree(int pid, bool force) { int sig = force ? SIGKILL : SIGTERM; bool any_success = false; bool any_permission_denied = false; + int skipped = 0; // Kill children first (reverse order) std::vector sorted_pids(pids_to_kill.begin(), pids_to_kill.end()); for (auto it = sorted_pids.rbegin(); it != sorted_pids.rend(); ++it) { + // Verify PID hasn't been reused before killing + if (auto st = start_times.find(*it); st != start_times.end()) { + if (!is_same_process(*it, st->second)) { + skipped++; + continue; + } + } if (::kill(*it, sig) == 0) { any_success = true; } else if (errno == EPERM) { From 30a033e40fda1d609922c9a38e19e382547c9e80 Mon Sep 17 00:00:00 2001 From: Dmitry Reznitsky Date: Sun, 8 Feb 2026 00:13:36 +0100 Subject: [PATCH 08/11] Document destruction ordering constraints for DataStore and providers Add comments in main.cpp and main_tui.cpp explaining why declaration order matters (C++ destroys locals in reverse order, so providers must be declared before DataStore). Also document the lifetime contract on DataStore's constructor. Co-Authored-By: Claude Opus 4.6 --- src/core/services/data_store.hpp | 5 ++++- src/main.cpp | 11 +++++++---- src/main_tui.cpp | 10 +++++++--- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/core/services/data_store.hpp b/src/core/services/data_store.hpp index 2bea79b..ef5d42e 100644 --- a/src/core/services/data_store.hpp +++ b/src/core/services/data_store.hpp @@ -64,7 +64,10 @@ struct DataSnapshot { class DataStore { public: - // Constructor with dependency injection for platform abstraction + // Constructor with dependency injection for platform abstraction. + // LIFETIME: The provided pointers must remain valid for the lifetime of + // this DataStore. The destructor calls stop() which joins the background + // thread, so callers must ensure providers outlive this object. DataStore(IProcessDataProvider* process_provider, ISystemDataProvider* system_provider); ~DataStore(); diff --git a/src/main.cpp b/src/main.cpp index 3c147ca..a23a574 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -21,17 +21,20 @@ int main([[maybe_unused]] int argc, [[maybe_unused]] char* argv[]) { } try { - // Create platform-specific providers (owned here in main) + // Create platform-specific providers (owned here in main). + // IMPORTANT: Declaration order matters for destruction safety. + // C++ destroys locals in reverse declaration order, so: + // 1. app is destroyed first (stops UI loop) + // 2. data_store is destroyed next (joins background thread) + // 3. providers are destroyed last (safe, no longer referenced) + // Do NOT reorder these declarations without verifying destruction safety. const auto process_provider = pex::make_process_data_provider(); const auto details_provider = pex::make_details_data_provider(); const auto system_provider = pex::make_system_data_provider(); const auto killer = pex::make_process_killer(); - // Create DataStore - the data layer that can be shared across UIs pex::DataStore data_store(process_provider.get(), system_provider.get()); - // Create and run the ImGui application (UI layer) - // ImGuiApp does not own these resources - they're managed here pex::ImGuiApp app(&data_store, system_provider.get(), details_provider.get(), killer.get()); instance.set_raise_callback([&app]() { diff --git a/src/main_tui.cpp b/src/main_tui.cpp index f64f419..e5c7ca7 100644 --- a/src/main_tui.cpp +++ b/src/main_tui.cpp @@ -15,16 +15,20 @@ int main([[maybe_unused]] int argc, [[maybe_unused]] char* argv[]) { sigaction(SIGCHLD, &sa, nullptr); try { - // Create platform-specific providers (owned here in main) + // Create platform-specific providers (owned here in main). + // IMPORTANT: Declaration order matters for destruction safety. + // C++ destroys locals in reverse declaration order, so: + // 1. app is destroyed first (stops UI loop, restores terminal) + // 2. data_store is destroyed next (joins background thread) + // 3. providers are destroyed last (safe, no longer referenced) + // Do NOT reorder these declarations without verifying destruction safety. const auto process_provider = pex::make_process_data_provider(); const auto details_provider = pex::make_details_data_provider(); const auto system_provider = pex::make_system_data_provider(); const auto killer = pex::make_process_killer(); - // Create DataStore - the data layer that can be shared across UIs pex::DataStore data_store(process_provider.get(), system_provider.get()); - // Create and run the TUI application (UI layer) pex::TuiApp app(&data_store, system_provider.get(), details_provider.get(), killer.get()); app.run(); return 0; From 7726b4e95972ce3e005224ea671c59c4fb266022 Mon Sep 17 00:00:00 2001 From: Dmitry Reznitsky Date: Sun, 8 Feb 2026 11:35:24 +0100 Subject: [PATCH 09/11] Fix Solaris CI: handle already-installed packages in pkg install Solaris pkg install returns exit code 4 when a package is already installed ("no updates necessary"). Add || true to each pkg install in the prepare step to prevent the VM's shell from aborting on this non-error condition. Co-Authored-By: Claude Opus 4.6 --- .github/workflows/cmake-multi-platform.yml | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/.github/workflows/cmake-multi-platform.yml b/.github/workflows/cmake-multi-platform.yml index 6a8a146..c968ce7 100644 --- a/.github/workflows/cmake-multi-platform.yml +++ b/.github/workflows/cmake-multi-platform.yml @@ -116,17 +116,17 @@ jobs: uses: vmactions/solaris-vm@v1 with: prepare: | - pkg install developer/gcc - pkg install developer/build/cmake - pkg install developer/build/gnu-make - pkg install system/header - pkg install x11/header - pkg install x11/library/libx11 - pkg install x11/library/libxrandr - pkg install x11/library/libxinerama - pkg install x11/library/libxcursor - pkg install x11/library/libxi - pkg install library/mesa + pkg install developer/gcc || true + pkg install developer/build/cmake || true + pkg install developer/build/gnu-make || true + pkg install system/header || true + pkg install x11/header || true + pkg install x11/library/libx11 || true + pkg install x11/library/libxrandr || true + pkg install x11/library/libxinerama || true + pkg install x11/library/libxcursor || true + pkg install x11/library/libxi || true + pkg install library/mesa || true run: | export PATH="/usr/gcc/*/bin:$PATH" cmake -B build -DCMAKE_BUILD_TYPE=Release -DBUILD_NCURSES=ON From f8b9359604368a3af5d0385ce33b50595a1e6dbb Mon Sep 17 00:00:00 2001 From: Dmitry Reznitsky Date: Sun, 8 Feb 2026 11:41:03 +0100 Subject: [PATCH 10/11] Fix Solaris CI: add git package for FetchContent clone CMake's FetchContent needs git to clone GLFW. The Solaris VM image previously included git but the current image does not. Co-Authored-By: Claude Opus 4.6 --- .github/workflows/cmake-multi-platform.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/cmake-multi-platform.yml b/.github/workflows/cmake-multi-platform.yml index c968ce7..8559035 100644 --- a/.github/workflows/cmake-multi-platform.yml +++ b/.github/workflows/cmake-multi-platform.yml @@ -117,6 +117,7 @@ jobs: with: prepare: | pkg install developer/gcc || true + pkg install developer/versioning/git || true pkg install developer/build/cmake || true pkg install developer/build/gnu-make || true pkg install system/header || true From 8fbc9a915e75fa686c3586c19c01e2fb208c6bb4 Mon Sep 17 00:00:00 2001 From: Dmitry Reznitsky Date: Sun, 8 Feb 2026 11:52:12 +0100 Subject: [PATCH 11/11] Fix ncurses build on Solaris: use CMAKE_MAKE_PROGRAM instead of make Solaris's default make is dmake, which doesn't support GNU make's -j flag syntax. Use CMAKE_MAKE_PROGRAM which resolves to gmake on Solaris (since CMake uses GNU make as its generator). Co-Authored-By: Claude Opus 4.6 --- CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 5a1c50a..b2cdfde 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -240,8 +240,8 @@ if(NOT NCURSES_FOUND AND BUILD_TUI AND BUILD_NCURSES) --without-ada --enable-widec --disable-stripping - BUILD_COMMAND make - INSTALL_COMMAND make install + BUILD_COMMAND ${CMAKE_MAKE_PROGRAM} + INSTALL_COMMAND ${CMAKE_MAKE_PROGRAM} install BUILD_IN_SOURCE 1 )