Save the backtrace when shutting down the DE.
to finish: Load the backtraces again in a nice UI.
Details
Details
Diff Detail
Diff Detail
- Repository
- R871 DrKonqi
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Comment Actions
Concept: +1
One question, when do we create a DBusInterfaceLauncher instance. Is it in time here?
Without this startkde won't know there's a drkonqi it should be waiting for and then whether you finish saving or not becomes racey.
| src/drkonqi.cpp | ||
|---|---|---|
| 215 | Can we avoid littering the home directory. Somewhere in .cache might make sense, as if it gets lost it's not too important. I've also been told it's more correct to use "/" instead of QDir::separator. | |
Comment Actions
As mentioned in D22323 the stuff in main.cpp needs putting in a class.
What's the rationale behind this change? Isn't this use case covered by coredumpd and the like?
Comment Actions
What's the rationale behind this change? Isn't this use case covered by coredumpd and the like?
Well, by that logic we wouldn't have any drkonqi...
- crashes on shutdown are currently weird.
A dialog appears and blocks on logout, on shutdown it's a random race. The crash dialog sometimes appears as an SNI which will get missed, sometimes a window on a different VD or different activity.
- with a tiny bit of work kwin_wayland crashes will get a bug reporter, which would be an amazing leap forward.
Comment Actions
I don't see how.
Drkonqi is and interactive crash reporter, coredumpd is a non-interactive crash catcher. What the diff is doing is taking away the interactivity of drkonqi in certain cases, and that looks to me like we are simply replicating coredumpd at this point. Specifically the diff adds nothing on top of what coredumpd already does, in fact it's worse because we currently don't capture cores, so you couldn't retrace our saved trace with possibly missing debug symbols.
The interactive way of handling a crash on logout would be to show drkonqi and let the user file a bug report. Now I understand this isn't all that reliable, so I would discard the crash and just exit under the assumption that a kernel core pattern (e.g. coredumpd) will deal with the crash.
If you are adamant about this diff I think we need at least a cleanup system to enforce a hard limit on how many traces we keep around, and/or traces of how many logins ago, and/or how many MiB they may consume. Otherwise people are going to get annoyed when there disk is full in a year because the ffmpegthumbnailer crashed a gazillion times ^^
And perhaps more ideally, albeit not blocking this diff, we should look at reshuffling the architecture so on next login drkonqi auto starts with a list of crashes and the ability to file reports for them.
| src/drkonqi.cpp | ||
|---|---|---|
| 225 | Needs to include pid. I can have multiple kwrites running and they could all be crashing. | |
Comment Actions
And perhaps more ideally, albeit not blocking this diff, we should look at reshuffling the architecture so on next login drkonqi auto starts with a list of crashes and the ability to file reports for them.
That second part is in the works. I had a working version but I did not like the outcome. Still, they can be separate reviews.
Comment Actions
- Add pid information to the log files
- Store only the last 10 files in the cache directory
Comment Actions
some style fixes then ship it plz 👍
| src/drkonqi.cpp | ||
|---|---|---|
| 214 | & goes to the right of the space; curly brace goes on next line for multi-line function bodies | |
| 217 | This doesn't necessarily need changing, but I want to point out that this and the following two lines are convoluted, they are simply for (int i = fileList.size(); i <= 10; --i) {} if I am reading this right. | |
| 219 | space between while and brace | |
| 232 | space between if and brace | |