-
-
Notifications
You must be signed in to change notification settings - Fork 52
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
Avoid fatal errors on specific modules #401
base: master
Are you sure you want to change the base?
Conversation
No need to exit the whole MPM process when the module doesn't work.
How do I test this given I too never see errors like this?
|
@lukefromdc First off, verifying it doesn't introduce any regression is a start :) Otherwise indeed I have no idea how to make the errors happen, short of modifying the code. Maybe @mokraemer could try this PR and see if that helps? |
It is hard for me to verify anything in mate-power-manager doesn't introduce any regressions. I would
have to temporarily transfer most of my work to the laptop to test it long enough, and I normally disable
things like suspending on closing the lid. A lot of settings would have to change.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NOT tested for function as that cannot be done without a way to invoke the errors, but I got a clean build and looking at the code I see we are just removing g_error which calls abort() and replacing it with g_warning, g_critical (which does not exit by default but can be told to at runtime) and in one case removing a return statement after one of these since we are not aborting.
None of this should have any effect on mate-power-manager when running normally, all it does is remove instructions to abort on encountering an error. Only a test by someone who is getting these errors can determine if not aborting on them produces worse problems such as a potentially exploitable segfault.
I see on another users machine a bunch of: I would guess this patch prevents those, please merge it |
Please check systemd version of the users machine. Patchlevel >=153-24 introduced a bug. |
systemd-256.6 |
I guess it has the same bug, as the patch is backported to the stable branch |
I think mate-power-manager should not crash regardless of bug in systemd |
true, but it will not work. As it can't connect to dbus |
That is better than producing coredumps and filling up the disk. |
most desktop set core files to zero. |
So to be clear, this patch stopped mate-power-manager from crashing which was the goal. |
@joakim-tjernlund I am not fully on your side. If you patch out this behaviour, we propably would not have found it and still have the problem as no one wanted to debug the issue. Most systems do not generate core files - they are disabled systemwide. So I suggest you do disable them in your system. Which leads to better results. |
An app can choose to recover or exit gracefully if external input is faulty but not crash. Crashing here is like your editor SEGV if you type the wrong cmd to it and that is not OK. |
no that is quite different. In case of my editor, almost all of them crash, if e.g. the filesystem is inaccessable - which is compareable to the systemd failure. |
No need to exit the whole MPM process if a module encounters a failure; otherwise the whole thing goes down.
Fixes #400.