Skip to content
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

ENH: Add process_name key to support broken/missing _NET_WM_PID #28

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,11 @@ Install binary package for your GNU/Linux distribution:
#### From Source

```bash
# Install dependencies, namely GLib, Libwnck, procps
Copy link
Author

@novahahn novahahn Oct 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know the package name(s) for RPM distros

# Install dependencies, namely GLib, Libwnck, libprocps, procps
# on Debian / Ubuntu / Mint:
sudo apt install libglib2.0-dev \
libwnck-3-dev \
libprocps-dev \
procps \
make cmake gcc pkg-config

Expand Down
5 changes: 5 additions & 0 deletions data/xsuspender.conf
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@
# # Also suspend descendant processes that match this regex.
# suspend_subtree_pattern = .
#
# # Assume that all windows matching this rule belong to the
# # process with this name. This is a workaround for
# # missing or incorrect _NET_WM_PID values.
novahahn marked this conversation as resolved.
Show resolved Hide resolved
# process_name = ...
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should in fact mention this matches cmdlines, so I wonder if we couldn't have a better expressed name, such as process_cmdline_matches?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And then need to explain this is being matched as literal strings.

Copy link
Author

@novahahn novahahn Oct 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like process_cmdline_matches since we don't match on the entire cmdline. How about program_basename_matches? That is what htop calls it.

Copy link
Owner

@kernc kernc Oct 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't match on the entire cmdline

I missed that. Wouldn't it be more versatile to match full cmdlines? process_... in either case, probably.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there's a good way to match on the full cmdline. Contains matchers would be too broad. Prefix and exact matchers would be too cumbersome. I don't think being able to match on the path to the executable is worth it. Regex could work, but I think that would be error-prone especially when paths contain spaces.

Matching on the process name + args would however be an improvement, I think.

#
# # Whether to apply the rule only when on battery power.
# only_on_battery = true
#
Expand Down
6 changes: 6 additions & 0 deletions doc/xsuspender.1
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,12 @@ This setting only applies when
.B send_signals
is also true.
.TP
.BR process_name
If provided, XSuspender will ignore _NET_WM_PID and instead detect
the process a window matching this rule belongs to by looking for
a process with this name. There should be only one process with this
name, not including its child processes.
novahahn marked this conversation as resolved.
Show resolved Hide resolved
.TP
.BR exec_suspend
.TQ
.BR exec_resume
Expand Down
5 changes: 3 additions & 2 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,20 @@ find_package (PkgConfig REQUIRED)

pkg_check_modules (GLIB REQUIRED glib-2.0)
pkg_check_modules (WNCK REQUIRED libwnck-3.0)
pkg_check_modules (PROCPS REQUIRED libprocps)

add_definitions (-DWNCK_I_KNOW_THIS_IS_UNSTABLE
-D_POSIX_SOURCE
-DG_LOG_DOMAIN="xsuspender")

add_compile_options (${GLIB_CFLAGS} ${WNCK_CFLAGS})
add_compile_options (${GLIB_CFLAGS} ${WNCK_CFLAGS} ${PROCPS_CFLAGS})

file (GLOB SOURCES "*.[ch]")
add_executable (${PROJECT_NAME} ${SOURCES})

# pkg-config recipe for libwnck may be a bit overzealos; trim linked libs to essentials
set (CMAKE_EXE_LINKER_FLAGS "-Wl,--as-needed -Wl,--no-undefined")

target_link_libraries (${PROJECT_NAME} PRIVATE ${GLIB_LDFLAGS} ${WNCK_LDFLAGS})
target_link_libraries (${PROJECT_NAME} PRIVATE ${GLIB_LDFLAGS} ${WNCK_LDFLAGS} ${PROCPS_LDFLAGS})

install (TARGETS ${PROJECT_NAME} DESTINATION bin)
7 changes: 7 additions & 0 deletions src/config.c
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,9 @@ read_section (GKeyFile *file,
str = g_key_file_get_value (file, section, CONFIG_KEY_SUBTREE_PATTERN, &err);
if (! is_error (&err)) reassign_str (&rule->subtree_pattern, str);

str = g_key_file_get_value (file, section, CONFIG_KEY_PROCESS_NAME, &err);
if ( !is_error (&err)) reassign_str (&rule->process_name, str);

char **argv;

argv = parse_command (file, section, CONFIG_KEY_EXEC_SUSPEND, &err);
Expand Down Expand Up @@ -190,6 +193,7 @@ debug_print_rule (Rule *rule)
"only_on_battery = %d\n"
"send_signals = %d\n"
"subtree_pattern = %s\n"
"process_name = %s\n"
"downclock_on_battery = %d\n"
"exec_suspend = %s\n"
"exec_resume = %s\n",
Expand All @@ -202,6 +206,7 @@ debug_print_rule (Rule *rule)
rule->only_on_battery,
rule->send_signals,
rule->subtree_pattern,
rule->process_name,
rule->downclock_on_battery,
(rule->exec_suspend ? rule->exec_suspend[2] : NULL),
(rule->exec_resume ? rule->exec_resume[2] : NULL)
Expand All @@ -222,6 +227,8 @@ parse_config ()

.subtree_pattern = NULL,

.process_name = NULL,

.downclock_on_battery = 0,

.delay = 10,
Expand Down
2 changes: 2 additions & 0 deletions src/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#define CONFIG_KEY_AUTO_ON_BATTERY "auto_suspend_on_battery"
#define CONFIG_KEY_SEND_SIGNALS "send_signals"
#define CONFIG_KEY_SUBTREE_PATTERN "suspend_subtree_pattern"
#define CONFIG_KEY_PROCESS_NAME "process_name"
#define CONFIG_KEY_DOWNCLOCK_ON_BATTERY "downclock_on_battery"
#define CONFIG_KEY_WM_NAME_CONTAINS "match_wm_name_contains"
#define CONFIG_KEY_WM_CLASS_CONTAINS "match_wm_class_contains"
Expand All @@ -26,6 +27,7 @@
CONFIG_KEY_AUTO_ON_BATTERY, \
CONFIG_KEY_SEND_SIGNALS, \
CONFIG_KEY_SUBTREE_PATTERN, \
CONFIG_KEY_PROCESS_NAME, \
CONFIG_KEY_DOWNCLOCK_ON_BATTERY, \
CONFIG_KEY_SUSPEND_DELAY, \
CONFIG_KEY_RESUME_EVERY, \
Expand Down
5 changes: 3 additions & 2 deletions src/entry.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@
#include <glib.h>
#include <libwnck/libwnck.h>

#include "proc.h"

WindowEntry*
xsus_window_entry_new (WnckWindow *window,
Rule *rule)
{
WindowEntry *entry = g_malloc (sizeof (WindowEntry));
entry->rule = rule;
entry->pid = wnck_window_get_pid (window);
entry->pid = xsus_window_get_pid (window);
entry->xid = wnck_window_get_xid (window);
entry->wm_name = g_strdup (wnck_window_get_name (window));
return entry;
Expand Down Expand Up @@ -44,7 +45,7 @@ xsus_entry_find_for_window_rule (WnckWindow *window,
{
// If suspending by signals, find entry by PID ...
if (rule->send_signals) {
pid_t pid = wnck_window_get_pid (window);
pid_t pid = xsus_window_get_pid (window);
for (; list; list = list->next) {
WindowEntry *entry = list->data;
if (entry->pid == pid)
Expand Down
7 changes: 4 additions & 3 deletions src/events.c
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include "entry.h"
#include "macros.h"
#include "proc.h"


void
Expand Down Expand Up @@ -69,7 +70,7 @@ windows_are_same_process (WnckWindow *w1,
Rule *rule;
return (WNCK_IS_WINDOW (w1) &&
WNCK_IS_WINDOW (w2) &&
wnck_window_get_pid (w1) == wnck_window_get_pid (w2) &&
xsus_window_get_pid (w1) == xsus_window_get_pid (w2) &&
(rule = xsus_window_get_rule (w1)) &&
rule->send_signals &&
rule == xsus_window_get_rule (w2));
Expand Down Expand Up @@ -104,7 +105,7 @@ pid_t
window_entry_get_pid (WindowEntry *entry)
{
WnckWindow *window = wnck_window_get (entry->xid);
return window && wnck_window_get_pid (window) == entry->pid ? entry->pid : 0;
return window && xsus_window_get_pid (window) == entry->pid ? entry->pid : 0;
}


Expand Down Expand Up @@ -411,7 +412,7 @@ on_update_downclocked_processes ()
continue;

// Skip any windows/PIDs we already know about
pid_t pid = wnck_window_get_pid (window);
pid_t pid = xsus_window_get_pid (window);
if (g_hash_table_contains (old_pids, GINT_TO_POINTER (pid)) ||
g_hash_table_contains (new_pids, GINT_TO_POINTER (pid)))
continue;
Expand Down
113 changes: 113 additions & 0 deletions src/proc.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
#include "proc.h"
#include <limits.h>
#include <proc/readproc.h>
#include "rule.h"


typedef struct state {
GTree *procs;
char *process_name;
// Types match readproc.h
int cand_pid;
unsigned long long cand_start_time; // NOLINT(runtime/int)
} state;


static
int
compare_ints (gconstpointer a, gconstpointer b, __attribute__((unused)) gpointer data)
{
return GPOINTER_TO_INT(a) - GPOINTER_TO_INT(b);
}


static
gboolean
cmdline_matches (char **cmdline, char *s)
{
if (cmdline == NULL)
return FALSE;

char *basename = g_path_get_basename (cmdline[0]);
gboolean result = g_strcmp0 (basename, s) == 0;
g_free (basename);
return result;
}


static
gboolean
traverse_procs (__attribute__((unused)) gpointer pid, proc_t *proc, state *state)
{
int cand_pid = 0;
unsigned long long cand_start_time = ULLONG_MAX; // NOLINT(runtime/int)

for (proc_t *cur = proc; cur; cur = g_tree_lookup(state->procs, GINT_TO_POINTER(cur->ppid))) {
if (cmdline_matches(cur->cmdline, state->process_name)) {
cand_pid = cur->tid;
cand_start_time = cur->start_time;
}
}

if (cand_pid != 0 && state->cand_pid != cand_pid) {
if (state->cand_pid != 0)
g_warning("Multiple processes named '%s': %u and %u",
state->process_name, state->cand_pid, cand_pid);

if (cand_start_time < state->cand_start_time ||
(cand_start_time == state->cand_start_time &&
cand_pid < state->cand_pid)) {
// Update
state->cand_start_time = cand_start_time;
state->cand_pid = cand_pid;
}
}

return FALSE;
}


static
GTree*
get_processes ()
{
GTree *result = g_tree_new_full(compare_ints, NULL, NULL, (GDestroyNotify) freeproc);
PROCTAB *pt = openproc (PROC_FILLARG | PROC_FILLSTAT);
proc_t *proc;

while ((proc = readproc (pt, NULL)))
g_tree_insert (result, GINT_TO_POINTER(proc->tid), proc);
novahahn marked this conversation as resolved.
Show resolved Hide resolved

closeproc (pt);
return result;
}


static
int
process_name_get_pid (char *process_name)
{
GTree *procs = get_processes();

state state;
state.procs = procs;
state.process_name = process_name;
state.cand_pid = 0;
state.cand_start_time = ULLONG_MAX;

g_tree_foreach(procs, (GTraverseFunc) traverse_procs, &state);
g_tree_destroy (procs);
return state.cand_pid;
}


int
xsus_window_get_pid (WnckWindow *window)
{
Rule *rule = xsus_window_get_rule (window);
char *process_name = rule ? rule->process_name : NULL;
int pid_by_process_name = process_name ? process_name_get_pid(process_name) : 0;
// Prefer getting the PID by process name so we can override _NET_WM_PID if it's wrong
// (e.g. when using flatpak)
return pid_by_process_name ? pid_by_process_name : wnck_window_get_pid(window);
}
7 changes: 7 additions & 0 deletions src/proc.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#ifndef XSUSPENDER_PROC_H
#define XSUSPENDER_PROC_H
#include <libwnck/libwnck.h>

int xsus_window_get_pid (WnckWindow *window);

#endif // XSUSPENDER_PROC_H
5 changes: 5 additions & 0 deletions src/rule.c
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <libwnck/libwnck.h>

#include "xsuspender.h"
#include "proc.h"

Rule*
xsus_rule_copy (Rule *orig)
Expand All @@ -15,6 +16,8 @@ xsus_rule_copy (Rule *orig)
rule->needle_wm_class = g_strdup (orig->needle_wm_class);
rule->needle_wm_class_group = g_strdup (orig->needle_wm_class_group);

rule->process_name = g_strdup (orig->process_name);

rule->exec_suspend = g_strdupv (orig->exec_suspend);
rule->exec_resume = g_strdupv (orig->exec_resume);

Expand All @@ -29,6 +32,8 @@ xsus_rule_free (Rule *rule)
g_free (rule->needle_wm_class_group);
g_free (rule->needle_wm_name);

g_free (rule->process_name);

g_strfreev (rule->exec_suspend);
g_strfreev (rule->exec_resume);

Expand Down
2 changes: 2 additions & 0 deletions src/rule.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ typedef struct Rule {

char* subtree_pattern;

char* process_name;

guint16 delay;
guint16 resume_every;
guint16 resume_for;
Expand Down
7 changes: 4 additions & 3 deletions src/xsuspender.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "events.h"
#include "exec.h"
#include "macros.h"
#include "proc.h"
#include "rule.h"


Expand Down Expand Up @@ -108,7 +109,7 @@ xsus_window_resume (WnckWindow *window)
if ((entry = xsus_entry_find_for_window_rule (window, rule, queued_entries))) {
g_debug ("Removing window %#lx (%d) from suspension queue: %s",
wnck_window_get_xid (window),
wnck_window_get_pid (window),
xsus_window_get_pid (window),
wnck_window_get_name (window));
queued_entries = g_slist_remove (queued_entries, entry);
xsus_window_entry_free (entry);
Expand All @@ -119,7 +120,7 @@ xsus_window_resume (WnckWindow *window)
if ((entry = xsus_entry_find_for_window_rule (window, rule, suspended_entries))) {
g_debug ("Resuming window %#lx (%d): %s",
wnck_window_get_xid (window),
wnck_window_get_pid (window),
xsus_window_get_pid (window),
wnck_window_get_name (window));
xsus_signal_continue (entry);
return;
Expand Down Expand Up @@ -150,7 +151,7 @@ xsus_window_suspend (WnckWindow *window)
g_debug ("Suspending window in %ds: %#lx (%d): %s",
rule->delay,
wnck_window_get_xid (window),
wnck_window_get_pid (window),
xsus_window_get_pid (window),
wnck_window_get_name (window));
WindowEntry *entry = xsus_window_entry_new (window, rule);
xsus_window_entry_enqueue (entry, rule->delay);
Expand Down