-
Notifications
You must be signed in to change notification settings - Fork 887
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
[rfc] kodi crashlogs #3657
[rfc] kodi crashlogs #3657
Conversation
Excellent. I've wanted this feature for a long while. Will test on Pi. |
That's excellent in deed! yes. +10! |
Works well on the Pi. Example output when simulating a seg fault can be seen here. It takes about 30 seconds to generate the crash log on a 1GHz Model B R-Pi (384MB/128MB memory split, with 128MB swap, and /storage on USB3 flash). It takes about 23 seconds to generate the crash log on a 1GHz Model B R-Pi (256MB/256MB memory split, with 128MB swap, and /storage on USB3 flash). |
What might be handy is having a sym link of /storage/.kodi/temp/kodi_crashlog to the last generated kodi_crashlog-*.log, so that it will be easier to get users to pastebin their last crashlog (getting them to try and work out the filename could be more trouble than it's worth). Edit: Ah, so this only ever keeps a maximum of one crash log. It still might be easier having a sym link, or just give the single crashlog file that will ever exist a consistent file name (similar to kodi.log) as the datestamp is present within the file. |
done @MilhouseVH |
Thanks. So it seems that release builds (without gdb) may accumulate an unlimited number of core dumps, as Perhaps only the last core should be retained in release builds (I'm assuming the core from a release build may still be useful to a developer if uploaded to dropbox etc.). |
nope. no coredumps will be stored at all if gdb is not installed. "max core file size" (ulimit -c) for every process/daemon is 0 by default, unless it is set via "ulimit -c" or LimitCORE=.. in daemon's systemd unit (kodi.service) https://github.com/OpenELEC/OpenELEC.tv/pull/3657/files#diff-28d0db85438a3e221913f31ca2c50c34R75 |
Smashing, thanks. |
ulimit -c unlimited | ||
fi | ||
|
||
/usr/lib/kodi/kodi.bin "$SAVED_ARGS" |
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.
This needs to be:
/usr/lib/kodi/kodi.bin $SAVED_ARGS
By quoting, the collection of args is treated as a single arg (and then you get no lircd with this commit)
@stefansaraev, had a report of this PR causing a problem when starting Retroarch - any ideas? http://forum.kodi.tv/showthread.php?tid=192380&pid=1855429#pid1855429 |
sure. when you call "systemctl stop kodi" - it basicaly commits suicide, as I have removed KillMode=process |
Yes, adding |
exactly. why do "kodi" addons want to stop kodi ? |
I guess that's a good question, but not one I can answer. |
because of loading RetroArch to play a Game |
@walterheisenberg I understand it but "systemctl stop kodi" from an addon that is run from kodi is plain wrong (EDIT: it has always been wrong. not just now..) people should use systemd-run to start their stuff in different cgroup, if they want to stop kodi and have their external stuff working example:
|
Yep, changing from
to
fixes the issue - thanks @stefansaraev |
OK, thank you guys for making that clear |
Next issue... :) When selecting "Power off" from the GUI system menu, I believe the guisettings.xml is being corrupted by this PR - when the device (a Pi) is next booted, the guisettings.xml file is 0 bytes:
I've noticed that with this PR, there are two threads saving the settings on shutdown:
but with the "old" kodi.service, there is only thread that saves the settings. Could the extra thread be trashing guisettings.xml? I can't reproduce the guisettings.xml corruption with the old kodi.service, but with this new kodi.service it happens frequently. |
I've done some more testing (using autostart.sh to log the size of guisettings.xml to a file) and it's not always zero bytes, for instance I've seen guisettings.xml with a size of 12288 when it should be 25953+. So I'd say there's a good chance the two threads are trampling on each other. |
yep @MilhouseVH confirmed. will fix it. thanks ;) EDIT: part of the problem is our "deadlock-on-exit fix". I forgot about it ;) |
@MilhouseVH last commit should fix it. |
@stefansaraev - initial testing looks good, many thanks! |
@stefansaraev: There is another problem with guisettings.xml, but it's completely the opposite of the previous problem as the settings are now never saved on exit (power off, reboot, systemctl stop/restart etc.). For instance, if you modify a setting with JSON, the modified setting will not be persisted in guisettings.xml on exit. Example:
Without this PR, the changes are correctly persisted on exit. Another way to confirm the problem is through the lack of any OnQuit message when kodi.bin shuts down - this message is not present when this PR is used:
but appears once when this PR is not used. |
huh. I am sure it was fine when I pushed the kill $MAINPID change. but I'll doublecheck tonight. |
Thanks. There's definitely something weird going on. I'd be inclined to think that kodi.bin is being killed before it can gracefully shut down and save the settings and complete any other exit tasks. There are however times when it will save the settings on exit, but this seems to be the exception rather than the rule. |
gotcha. that happens when one (me) reads the systemd docs between the lines. I was blind. if you are curious, here is what happened: systemd sends TERM to $MAINPID (kodi.sh) then TERM to all remaining processes in the cgroup (including kodi.bin). as kodi.sh exits immediately on TERM, the service is considered "clean stopped" and systemd continues the reboot/shutdown sequence. if kodi.bin had chance to save guisettings.xml - that was by pure luck (facepalm. I am blind). it never happens on "systemctl stop kodi". by design. but reboot/shutdown is different story. now kodi.sh waits for kodi.bin to exit. if it does not exit within the TimeoutStopSec interval. well... systemd sends KILL. it has been that way for long, to prevent "deadlocks" on shutdown/reboot and well I will update our sigterm handler patch to guard against double sigterm. but that's outside of the scope of this PR. |
@stefansaraev testing with the updates and there's still a problem in some of the shutdown scenarios.
So it looks like there's a problem saving settings when using the GUI power menu options. |
are you sure the changes to kodi.sh are applied or no modified kodi.service in .config ? or is there any rpi-specific patch that does "reboot from gui" in different way and might interfere? |
I'm sure the changes are there and there is no kodi.service in .config (except when I'm testing the old service).
Not that I'm aware of. d0f7237 which went in on the 14th did cause a problem with the Pi's "virtual suspend" - these patches are not yet upstream however I've tested a build without these "virual suspend" patches. I tested a build using only b7d591d plus d0f7237 and the latest PR3657/SIGTERM and it made no difference - still no settings saved with the new service/new SIGTERM patch, or the old service/new SIGTERM. It may have something to do with the change you made to the SIGTERM patch as I'm not seeing Here's four logs - two from my last test build #1215 (latest PR3657/latest SIGTERM) and two from the previous build #1214 (previous PR3657/previous SIGTERM). In all cases I'm enabling debug via json, then rebooting from the GUI. #1215, New Service (settings not saved on reboot):
#1215, Old Service (settings not saved on reboot):
#1214, New Service (settings not saved on reboot):
#1214, Old Service (settings saved on reboot):
With the #1215 build (old or new service, new SIGTERM) and #1214 build (new service, old SIGTERM) there is no Only with the #1214 build (old service, old SIGTERM) do I see the OnQuit and SIGTERM messages being processed/received. Given that the latest #1215 build no longer works correctly with the "old" service, I strongly suspect the updated SIGTERM handler is not working as expected. |
With 6597a82 (ie. SIGTERM update, reverted) this is now testing very well. A new build (ie. new service, old SIGTERM) seems to be working well - no duplicate attempts to save settings (causing corruption), and always one attempt to save settings on all of the following shutdowns:
Will test more, but thumbs up so far! Hopefully that's it nailed. :) |
now. after xbmc/xbmc#5895 we have a problem. a big one, that will affect lot of stuff (not only this PR). we will need: both tested and work. I'd prefer first one, but it's ugly... (XDG_DATA_HOME= ) the .xbmc -> .kodi move was enough pain. this xdg nonsense (sorry for the wording) will be even more pain.. so up to you. ping @sraue |
ok, xbmc/5895 was reverted. IMO we are good to go. when (or if..) it is added back upstream, we will take care, it is unrelated to this PR. @sraue any objections? this has been well tested and should be safe (for master only) |
yeah please commit if you are sure its ok, on master i will stick with helix for some time or readd support for building master or helix based on make options. master will become 5.2 soon (with kernel 3.18/19, systemd 218, python 2.7.9 and mesa 10.4) |
on ubuntu, when kodi crashes, it dumps a nice crashlog. crashlogs help us and kodi devs to find out WHY..
the kodi.sh script included in this PR is a shameless rip off, cleaned up a bit ;)
I have tested on x86_64 and so far all good (simulating crash via kill -SEGV). but this needs testing. on RPi/Cuboxi-i with slow sdcard it might take quite lot of time to dump a 300+ MB core. also sh*t may happen on netbooted clients (but we shouldnt care much for those, anyway) EDIT: oh, and those people who install x86_64 on usb sticks, too...
please dont merge (yet), this is for after OE 5 and should not go to 5.0 branch.
@sraue any comments?
//cc @fritsch