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

Conversation

novahahn
Copy link

@novahahn novahahn commented Oct 20, 2021

I tried to configure XSuspender for Steam:

[Steam]
match_wm_class_contains = Steam

This had no effect: XSuspender added and removed the main window to and from the suspension queue ("Suspending window in 5s: 0xa20002b (0): Steam/Removing window 0xa20002b (0) from suspension queue: Steam", but did not actually send any signals. I tried adding

exec_suspend = pkill -STOP steam
exec_resume = pkill -CONT steam

but these were not executed until I also added send_signals = false. But then, XSuspender also suspended Steam when I the settings window.

Using this patch, I'm currently testing the config

[Steam]
match_wm_class_contains = Steam
suspend_subtree_pattern = steam
process_name = steam
exec_suspend = ! pgrep -lf 'reaper SteamLaunch AppId' # Running games don't like it when Steam is suspended

which seems to work.


Although I don't use Flatpak for it, Steam does not set _NET_WM_PID at all. It uses multiple processes, but they all belong to a single subtree.

image

@@ -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

Copy link
Owner

@kernc kernc left a comment

Choose a reason for hiding this comment

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

Can you describe in a few words what issue this is fixing for you and how you use it?

Thanks!

data/xsuspender.conf Outdated Show resolved Hide resolved
doc/xsuspender.1 Outdated Show resolved Hide resolved
# # 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.
# 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.

src/proc.c Outdated Show resolved Hide resolved
src/proc.c Outdated Show resolved Hide resolved
src/proc.c Outdated Show resolved Hide resolved
src/proc.c Outdated Show resolved Hide resolved
@kernc
Copy link
Owner

kernc commented Oct 21, 2021

I'm still most readily waiting for your use case example! E.g. "while running Flatpak app Foo, it happens that ..." What are the set PIDs, process cmdlines, process tree? As the reader of this bug report/PR, I'd naturally like to see and understand the broken setup, without needing to investigate everything from the absolute beginning.

@novahahn
Copy link
Author

OK. I thought the implicit "use XSuspender when _NET_WM_PID is broken or missing" was sufficient :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants