This patch has it signal the disappearance of the outputModel and only then deletes it.
This gives the QML side of things a chance to clean up, specifically QQuickComboBox stores the model in a QVariant and crashes if you just delete its backing model.
Ideally the model instance itself would never be replaced but just reset and populated with new outputs.
Details
Details
- Reviewers
romangg davidedmundson - Group Reviewers
Plasma - Commits
- R104:2ab4c2f136c5: Gracefully replace outputModel
QQC2 ComboBox should probably be fixed to store it in a QPointer or something but this at least fixes the crash.
Opened KScreen KCM, changed something, hit "Reset", no crash
Diff Detail
Diff Detail
- Repository
- R104 KScreen
- Lint
Lint Skipped - Unit
Unit Tests Skipped
Comment Actions
But just destroying and recreating is much simpler. ;)
Regarding the change: if we release without delete afterwards we leak memory. Maybe deleteLater?
Comment Actions
But just destroying and recreating is much simpler. ;)
And causes crashes by leaving things on the QML side in an inconsistent state.
Not sure deleteLater will help you since GetConfigOperation() could take longer than that. We could perhaps delete the old model when the config operation finishes, but that might get ugly. I'll give it a shot.
Comment Actions
Urgh, so the order of events is:
KCMKScreen::~KCMKScreen() <-- deletes the config and exposed properties
KCModuleQML::~KCModuleQML() <-- tears down the UI, which now references dangling things.
I think thats's a sign of a more generic that it would be good if we could solve, maybe an explicit method in KCModuleQML we could call from our destructor. It sounds similar to that language translation crash.
For 5.17, change to .get() to address Roman's comment and then +1 from me.
Comment Actions
I call release() instead of get() so I reset the pointer to null so that when I signal outputModelChanged it returns null and QtQuick resets everything and only then I delete it.
Comment Actions
Ok, this comment explains why you do it this way. An important detail for understanding what's happening is the early return of KCMKScreen::outputModel(), where null is returned in case there is no m_config. Please reformulate the comment in the code so it's more easy to understand what's going on. Thanks for the fix!