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

Catch exception in call to psutil .cmdline() #413

Merged
merged 1 commit into from
Aug 10, 2022

Conversation

zoitrok
Copy link
Contributor

@zoitrok zoitrok commented Aug 1, 2022

There's a race condition where processes might disappear while filtering. The uncaught exception causes a crash.

See the following stack trace:
Traceback (most recent call last):
File "/usr/bin/auto-cpufreq", line 226, in
main()
File "/usr/lib/python3.10/site-packages/click/core.py", line 1130, in call
return self.main(*args, **kwargs)
File "/usr/lib/python3.10/site-packages/click/core.py", line 1055, in main
rv = self.invoke(ctx)
File "/usr/lib/python3.10/site-packages/click/core.py", line 1404, in invoke
return ctx.invoke(self.callback, **ctx.params)
File "/usr/lib/python3.10/site-packages/click/core.py", line 760, in invoke
return __callback(*args, **kwargs)
File "/usr/bin/auto-cpufreq", line 117, in main
running_daemon()
File "/usr/lib/python3.10/site-packages/auto_cpufreq/core.py", line 1190, in running_daemon
if is_running("auto-cpufreq", "--daemon"):
File "/usr/lib/python3.10/site-packages/auto_cpufreq/core.py", line 1175, in is_running
for s in filter(lambda x: program in x, p.cmdline()):
File "/usr/lib/python3.10/site-packages/psutil/init.py", line 684, in cmdline
return self._proc.cmdline()
File "/usr/lib/python3.10/site-packages/psutil/_pslinux.py", line 1668, in wrapper
raise NoSuchProcess(self.pid, self._name)
psutil.NoSuchProcess: process no longer exists (pid=669342)

@AdnanHodzic
Copy link
Owner

Nice catch and thank your contribution, but there's one problem.

On master branch changes, if auto-cpufreq daemon is installed (sudo auto-cpufreq --install), and I subsequently run:

sudo auto-cpufreq --live   

Using settings defined in /etc/auto-cpufreq.conf file

Note: You can quit live mode by pressing "ctrl+c"

------------------------ auto-cpufreq running ------------------------------

ERROR: auto-cpufreq is running in daemon mode.

Make sure to stop the daemon before running with --live or --monitor mode

or

sudo auto-cpufreq --monitor

Using settings defined in /etc/auto-cpufreq.conf file

Note: You can quit monitor mode by pressing "ctrl+c"

------------------------ auto-cpufreq running ------------------------------

ERROR: auto-cpufreq is running in daemon mode.

Make sure to stop the daemon before running with --live or --monitor mode

-------------------------------------------------------------------------------

But with your changes, if daemon is installed I won't get any of these errors and then we'll have duplicate (live, mointor) instance of auto-cpufreq running which will be counter productive.

There's a race condition where processes might disappear while filtering. The uncaught exception causes a crash.
@zoitrok zoitrok changed the title Break out filtering of processes from lambda to catch exceptions. Catch exception in call to psutil .cmdline() Aug 9, 2022
@zoitrok
Copy link
Contributor Author

zoitrok commented Aug 9, 2022

Looks like I was overcomplicating it a bit. I'm not so familiar with python nor lambdas.

This amended commit seems to do what it's supposed to with daemon running and trying a live session. (and probably as fast as the original function).

@AdnanHodzic
Copy link
Owner

Great, these changes make lot more sense! I also noticed there's a bug (unrelated to your PR) where user will be able to run multiple instances of sudo auto-cpufreq --live.

Again, thank you for your contribution and you made it at the perfect moment as today or tomorrow I'll make a new release in which of course you'll be credited for you work.

@AdnanHodzic AdnanHodzic merged commit f67e909 into AdnanHodzic:master Aug 10, 2022
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.

2 participants