[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    [kcmutils] src: KCMultiDialog: Fix crash when clicking OK
From:       Aurélien Gâteau <agateau () kde ! org>
Date:       2014-06-13 7:41:43
Message-ID: E1WvM7L-0002bv-Gz () scm ! kde ! org
[Download RAW message or body]

Git commit cc99fac3af737619ea220ccb7059f0d8f0623efa by Aurélien Gâteau.
Committed on 10/06/2014 at 14:40.
Pushed by gateau into branch 'master'.

KCMultiDialog: Fix crash when clicking OK

When one closes KCMultiDialog with OK, QDialog::finished() can be emitted
before the clicked() signal of the OK button. This causes the following
sequence to happen:

- KCMultiDialogPrivate::_k_dialogClosed
  * deletes the KCModule
    - KCModuleProxyPrivate::_k_moduleDestroyed
      * sets kcm to 0

- KCMultiDialog::slotOkClicked
  - KCMultiDialogPrivate::apply
    - KCModuleProxy::save
      - KCModuleProxyPrivate::realModule
        * notices kcm is 0, so recreates it
        * calls kcm->save()
          - KWinDesktopConfig::save()
            * crashes because it expects kcm->load() to have been called

To avoid this, trigger the cleanup code in closeEvent() rather than when
finished() is emitted, as we can be sure closeEvent() is always called
*after* the methods connected to the button box signals has executed.

REVIEW: 118639
BUG: 334624

M  +15   -15   src/kcmultidialog.cpp
M  +2    -1    src/kcmultidialog.h

http://commits.kde.org/kcmutils/cc99fac3af737619ea220ccb7059f0d8f0623efa

diff --git a/src/kcmultidialog.cpp b/src/kcmultidialog.cpp
index 478c25a..5be7595 100644
--- a/src/kcmultidialog.cpp
+++ b/src/kcmultidialog.cpp
@@ -199,20 +199,6 @@ void KCMultiDialogPrivate::_k_updateHeader(bool use, const QString &message)
     }
 }
 
-void KCMultiDialogPrivate::_k_dialogClosed()
-{
-    // qDebug() ;
-
-    /**
-     * If we don't delete them, the DBUS registration stays, and trying to load the KCMs
-     * in other situations will lead to "module already loaded in Foo," while to the user
-     * doesn't appear so(the dialog is hidden)
-     */
-    for (int i = 0; i < modules.count(); ++i) {
-        modules[ i ].kcm->deleteClient();
-    }
-}
-
 void KCMultiDialogPrivate::init()
 {
     Q_Q(KCMultiDialog);
@@ -238,7 +224,6 @@ void KCMultiDialogPrivate::init()
 
     q->setButtonBox(buttonBox);
 
-    q->connect(q, SIGNAL(finished(int)), SLOT(_k_dialogClosed()));
     q->connect(q, SIGNAL(currentPageChanged(KPageWidgetItem*,KPageWidgetItem*)),
                SLOT(_k_slotCurrentPageChanged(KPageWidgetItem*,KPageWidgetItem*)));
 
@@ -388,6 +373,21 @@ void KCMultiDialog::slotHelpClicked()
     }
 }
 
+void KCMultiDialog::closeEvent(QCloseEvent *event)
+{
+    Q_D(KCMultiDialog);
+    KPageDialog::closeEvent(event);
+
+    /**
+     * If we don't delete them, the DBUS registration stays, and trying to load the KCMs
+     * in other situations will lead to "module already loaded in Foo," while to the user
+     * doesn't appear so(the dialog is hidden)
+     */
+    Q_FOREACH(auto &proxy, d->modules) {
+        proxy.kcm->deleteClient();
+    }
+}
+
 KPageWidgetItem *KCMultiDialog::addModule(const QString &path, const QStringList &args)
 {
     QString complete = path;
diff --git a/src/kcmultidialog.h b/src/kcmultidialog.h
index 72e45a5..78b1625 100644
--- a/src/kcmultidialog.h
+++ b/src/kcmultidialog.h
@@ -126,6 +126,8 @@ protected:
 
     KCMultiDialogPrivate *const d_ptr;
 
+    void closeEvent(QCloseEvent *event) Q_DECL_OVERRIDE;
+
 protected Q_SLOTS:
     /**
      * This slot is called when the user presses the "Default" Button.
@@ -174,7 +176,6 @@ protected Q_SLOTS:
 private:
     Q_PRIVATE_SLOT(d_func(), void _k_slotCurrentPageChanged(KPageWidgetItem *, KPageWidgetItem *))
     Q_PRIVATE_SLOT(d_func(), void _k_clientChanged())
-    Q_PRIVATE_SLOT(d_func(), void _k_dialogClosed())
     Q_PRIVATE_SLOT(d_func(), void _k_updateHeader(bool use, const QString &message))
 };
 
[prev in list] [next in list] [prev in thread] [next in thread]