BUG:390774
Details
Details
- Reviewers
hein ivan - Group Reviewers
Plasma Frameworks
Compiled and plasmashell doesn't crash when open kicker
Diff Detail
Diff Detail
- Repository
- R159 KActivities Statistics
- Branch
- master
- Lint
No Linters Available - Unit
No Unit Test Coverage
Comment Actions
This is not really as simple as 'accept without a comment'.
I don't mind this patch to be merged, but keep in mind that it just turns users for which plasma crashes (for which we get an explicit message as to why) into users who do not have certain parts of Plasma working where the error message will get lost in the Plasma's log. Now, as a maintainer of said parts of Plasma, that is your call.
Comment Actions
@ivan So you wrote a method that can return null, but the call sites aren't actually prepared to handle that gracefully? :) But I was waiting for your review for that context.
Comment Actions
Well, in a library that 99% relies on a database, having no database is not something that can be gracefully handled (the returned nullptr is asserted on as well in the function that calls ::instance in ResultSet :) - because of this, I'm changing to 'Request Changes').
Graceful handling instead of qFatal/Q_ASSERT could be implemented so that all models and queries return empty sets. Which would be useless, but as I said, I don't have anything against that.
When we get the bug reports about empty favourites, there will be an additional test step "Hi, do you have this message in the plasmashell output 'Database can not be opened...'" :)
Comment Actions
@ivan When database is broken additionaly kactivitymanagerd is in endless loop of creating and closing processes. Each KF5 application prints to console message: "KActivities: FATAL ERROR: Failed to contact the activity manager daemon"
Maybe it would be good to display dialog from kactivitymanagerd and ask user to fix (remove and create again) database instead of creating new processes infinitialy? It would fix the source of the problem which is broken database (I only wonder why the database is broken).
I think that for now it's more important to fix crash and then help user to fix database. Additionally at this moment I think I don't have good C++ skills to fix that another way :(
Comment Actions
The fix needs to be a bit more complex. We can not really rely on the user to fix the database.
The current plan is to have kamd backup the important parts of the database from time to time, and to recreate the database when needed.
I'll see whether I can roll out a crash=>fail-with-a-message patch for the LTS.
The current implementation relied on my (wrong) assumption that sqlite is very stable and that it can not go berserk and corrupt the database. :)