[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-core-devel
Subject: Re: [PATCH] kwalletd, get rid of delayed messages
From: Michael Leupold <lemma () confuego ! org>
Date: 2008-09-20 17:53:53
Message-ID: 200809201953.54076.lemma () confuego ! org
[Download RAW message or body]
On Saturday 20 September 2008, Thiago Macieira wrote:
> Michael Leupold wrote:
> >following a suggestion by Thiago I propose the attached patch to
> > kwalletd. It gets rid of delaying messages by changing how the a wallet
> > SHOULD be opened: - client sends an openAsync methodcall
> >- server returns a transaction identifier and starts handling the
> > transaction - client listens to the server's walletAsyncOpened signal
> > (which will be emitted on success and failure)
> Sounds like the best solution.
> Thanks for doing that.
I found one major bug that made the daemon crash on cancelling entering the
password. The client code is ready as well now.
One question about binary compatibility:
Besides adding two private slots I also removed two which are no longer
needed. That should be ok, shouldn't it?
Ok to commit?
Regards,
Michael
["kwalletd-asyncopen-2.diff" (text/x-patch)]
Index: kdebase/runtime/kwalletd/kwalletd.cpp
===================================================================
--- kdebase/runtime/kwalletd/kwalletd.cpp (Revision 862863)
+++ kdebase/runtime/kwalletd/kwalletd.cpp (Arbeitskopie)
@@ -48,30 +48,48 @@
#include <QtGui/QTextDocument> // Qt::escape
#include <QtCore/QRegExp>
#include <QtCore/QTimer>
+#include <QtCore/QEventLoop>
#include <assert.h>
#include "kwalletdadaptor.h"
#include "kwalletsynctimer.h"
+#include "kwalletopenloop.h"
class KWalletTransaction {
+
public:
- KWalletTransaction() {
- tType = Unknown;
+ KWalletTransaction() : tType(Unknown), tId(nextTransactionId) {
+ nextTransactionId++;
+ // make sure the id is never < 0 as that's used for the
+ // error conditions.
+ if (nextTransactionId < 0) {
+ nextTransactionId = 0;
+ }
}
~KWalletTransaction() {
}
- enum Type { Unknown, Open, ChangePassword, OpenFail };
- QDBusMessage msg;
+ enum Type {
+ Unknown,
+ Open,
+ ChangePassword,
+ OpenFail
+ };
Type tType;
QString appid;
qlonglong wId;
QString wallet;
bool modal;
+ int tId; // transaction id
+
+ private:
+ static int nextTransactionId;
};
+int KWalletTransaction::nextTransactionId = 0;
+
KWalletD::KWalletD()
: QObject(0), _failed(0) {
srand(time(0));
@@ -150,7 +168,7 @@
// Process remaining transactions
KWalletTransaction *xact;
while (!_transactions.isEmpty()) {
- xact = _transactions.first();
+ xact = _transactions.takeFirst();
int res;
assert(xact->tType != KWalletTransaction::Unknown);
@@ -163,10 +181,8 @@
// should not produce multiple password
// dialogs on a failure
if (res < 0) {
- QList<KWalletTransaction *>::iterator it = _transactions.begin();
- Q_ASSERT(*it == xact);
- ++it;
- for (; it != _transactions.end(); ++it) {
+ QList<KWalletTransaction *>::iterator it;
+ for (it = _transactions.begin(); it != _transactions.end(); ++it) {
KWalletTransaction *x = *it;
if (xact->appid == x->appid && x->tType == KWalletTransaction::Open
&& x->wallet == xact->wallet && x->wId == xact->wId) {
@@ -174,56 +190,29 @@
}
}
}
+
+ // emit the AsyncOpened signal as a reply
+ emit walletAsyncOpened(xact->tId, res);
break;
case KWalletTransaction::OpenFail:
- res = -1;
+ // emit the AsyncOpened signal with an invalid handle
+ emit walletAsyncOpened(xact->tId, -1);
break;
case KWalletTransaction::ChangePassword:
doTransactionChangePassword(xact->appid, xact->wallet, xact->wId);
// fall through - no return
default:
- _transactions.removeAll(xact);
- continue;
+ break;
}
-
- if (xact->tType != KWalletTransaction::ChangePassword) {
- QDBusConnection::sessionBus().send(xact->msg.createReply(res));
- }
- _transactions.removeAll(xact);
+
+ delete xact;
}
processing = false;
}
-#if 0
-void KWalletD::openAsynchronous(const QString& wallet, const QByteArray& \
returnObject, uint wId, const QString& appid) {
- DCOPClient *dc = callingDcopClient();
- if (!dc) {
- return;
- }
- if (!_enabled ||
- !QRegExp("^[A-Za-z0-9]+[A-Za-z0-9\\s\\-_]*$").exactMatch(wallet)) {
- DCOPRef(appid, returnObject).send("walletOpenResult", -1);
- return;
- }
- KWalletTransaction *xact = new KWalletTransaction;
-
- xact->appid = appid;
- xact->wallet = wallet;
- xact->wId = wId;
- xact->modal = false;
- xact->tType = KWalletTransaction::Open;
- xact->returnObject = returnObject;
- _transactions.append(xact);
-
- DCOPRef(appid, returnObject).send("walletOpenResult", 0);
-
- QTimer::singleShot(0, this, SLOT(processTransactions()));
-}
-#endif
-
int KWalletD::openPath(const QString& path, qlonglong wId, const QString& appid) {
if (!_enabled) { // guard
return -1;
@@ -234,12 +223,22 @@
return rc;
}
+int KWalletD::open(const QString& wallet, qlonglong wId, const QString& appid) {
+ int tId = openAsync(wallet, wId, appid);
+ if (tId < 0) {
+ return tId;
+ }
+
+ // wait for the open-transaction to be processed
+ KWalletOpenLoop loop(this);
+ return loop.waitForAsyncOpen(tId);
+}
-int KWalletD::open(const QString& wallet, qlonglong wId, const QString& appid, const \
QDBusMessage &msg) { +int KWalletD::openAsync(const QString& wallet, qlonglong wId, \
const QString& appid) { if (!_enabled) { // guard
return -1;
}
-
+
if (!QRegExp("^[A-Za-z0-9]+[A-Za-z0-9\\s\\-_]*$").exactMatch(wallet)) {
return -1;
}
@@ -247,8 +246,6 @@
KWalletTransaction *xact = new KWalletTransaction;
_transactions.append(xact);
- msg.setDelayedReply(true);
- xact->msg = msg;
xact->appid = appid;
xact->wallet = wallet;
xact->wId = wId;
@@ -256,7 +253,8 @@
xact->tType = KWalletTransaction::Open;
QTimer::singleShot(0, this, SLOT(processTransactions()));
checkActiveDialog();
- return 0; // process later
+ // opening is in progress. return the transaction number
+ return xact->tId;
}
// Sets up a dialog that will be shown by kwallet.
@@ -563,11 +561,10 @@
}
-void KWalletD::changePassword(const QString& wallet, qlonglong wId, const QString& \
appid, const QDBusMessage& msg) { +void KWalletD::changePassword(const QString& \
wallet, qlonglong wId, const QString& appid) { KWalletTransaction *xact = new \
KWalletTransaction;
//msg.setDelayedReply(true);
- xact->msg = msg;
xact->appid = appid;
xact->wallet = wallet;
xact->wId = wId;
Index: kdebase/runtime/kwalletd/kwalletd.h
===================================================================
--- kdebase/runtime/kwalletd/kwalletd.h (Revision 862863)
+++ kdebase/runtime/kwalletd/kwalletd.h (Arbeitskopie)
@@ -51,15 +51,13 @@
bool isEnabled() const;
// Open and unlock the wallet
- int open(const QString& wallet, qlonglong wId, const QString& appid, const \
QDBusMessage& msg); + int open(const QString& wallet, qlonglong wId, const QString& \
appid);
// Open and unlock the wallet with this path
int openPath(const QString& path, qlonglong wId, const QString& appid);
- // Asynchronous open - must give the object to return the handle
- // to.
- // disabled -thiago
- //virtual void openAsynchronous(const QString& wallet, const QByteArray& \
returnObject, uint wId, const QString& appid); + // Open the wallet asynchronously
+ int openAsync(const QString& wallet, qlonglong wId, const QString& appid);
// Close and lock the wallet
// If force = true, will close it for all users. Behave. This
@@ -82,7 +80,7 @@
QStringList users(const QString& wallet) const;
// Change the password of this wallet
- void changePassword(const QString& wallet, qlonglong wId, const QString& appid, \
const QDBusMessage& msg); + void changePassword(const QString& wallet, qlonglong \
wId, const QString& appid);
// A list of all wallets
QStringList wallets() const;
@@ -148,6 +146,7 @@
void screenSaverChanged(bool);
Q_SIGNALS:
+ void walletAsyncOpened(int id, int handle); // used to notify KWallet::Wallet
void walletListDirty();
void walletCreated(const QString& wallet);
void walletOpened(const QString& wallet);
Index: kdebase/runtime/kwalletd/kwalletopenloop.cpp
===================================================================
--- kdebase/runtime/kwalletd/kwalletopenloop.cpp (Revision 0)
+++ kdebase/runtime/kwalletd/kwalletopenloop.cpp (Revision 0)
@@ -0,0 +1,46 @@
+// -*- indent-tabs-mode: t; tab-width: 4; c-basic-offset: 4; -*-
+/*
+ This file is part of the KDE libraries
+
+ Copyright (c) 2008 Michael Leupold <lemma@confuego.org>
+
+ This library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Library General Public
+ License as published by the Free Software Foundation; either
+ version 2 of the License, or (at your option) any later version.
+
+ This library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Library General Public License for more details.
+
+ You should have received a copy of the GNU Library General Public License
+ along with this library; see the file COPYING.LIB. If not, write to
+ the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ Boston, MA 02110-1301, USA.
+
+*/
+
+#include "kwalletopenloop.h"
+#include "kwalletd.h"
+
+int KWalletOpenLoop::waitForAsyncOpen(int tId)
+{
+ transaction = tId;
+ connect(wallet, SIGNAL(walletAsyncOpened(int, int)),
+ SLOT(walletAsyncOpened(int, int)));
+ exec();
+ disconnect(this, SLOT(walletAsyncOpened(int, int)));
+ return result;
+}
+
+void KWalletOpenLoop::walletAsyncOpened(int tId, int handle)
+{
+ // if our transaction finished, stop waiting
+ if (tId == transaction) {
+ result = handle;
+ quit();
+ }
+}
+
+#include "kwalletopenloop.moc"
Index: kdebase/runtime/kwalletd/kwalletopenloop.h
===================================================================
--- kdebase/runtime/kwalletd/kwalletopenloop.h (Revision 0)
+++ kdebase/runtime/kwalletd/kwalletopenloop.h (Revision 0)
@@ -0,0 +1,50 @@
+// -*- indent-tabs-mode: t; tab-width: 4; c-basic-offset: 4; -*-
+/*
+ This file is part of the KDE libraries
+
+ Copyright (c) 2008 Michael Leupold <lemma@confuego.org>
+
+ This library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Library General Public
+ License as published by the Free Software Foundation; either
+ version 2 of the License, or (at your option) any later version.
+
+ This library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Library General Public License for more details.
+
+ You should have received a copy of the GNU Library General Public License
+ along with this library; see the file COPYING.LIB. If not, write to
+ the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ Boston, MA 02110-1301, USA.
+
+*/
+
+#ifndef KWALLETOPENLOOP_H
+#define KWALLETOPENLOOP_H
+
+#include <QtCore/QEventLoop>
+
+class KWalletD;
+
+class KWalletOpenLoop : public QEventLoop {
+ Q_OBJECT
+
+ public:
+ KWalletOpenLoop(KWalletD* w, QObject* parent = 0)
+ : QEventLoop(parent), wallet(w) {}
+
+ // returns the open handle
+ int waitForAsyncOpen(int tId);
+
+ public slots:
+ void walletAsyncOpened(int tId, int handle);
+
+ private:
+ KWalletD* wallet;
+ int transaction;
+ int result;
+};
+
+#endif
Index: kdebase/runtime/kwalletd/kwalletdadaptor.h
===================================================================
--- kdebase/runtime/kwalletd/kwalletdadaptor.h (Revision 862863)
+++ kdebase/runtime/kwalletd/kwalletdadaptor.h (Arbeitskopie)
@@ -41,15 +41,16 @@
{ return parent->isEnabled(); }
// Open and unlock the wallet
- inline int open(const QString& wallet, qlonglong wId, const QString& appid, \
const QDBusMessage& msg)
- { return parent->open(wallet, wId, appid, msg); }
+ inline int open(const QString& wallet, qlonglong wId, const QString& appid)
+ { return parent->open(wallet, wId, appid); }
// Open and unlock the wallet with this path
inline int openPath(const QString& path, qlonglong wId, const QString& appid)
{ return parent->openPath(path, wId, appid); }
- // disabled -thiago
- //virtual void openAsynchronous(const QString& wallet, const QByteArray& \
returnObject, uint wId); + // Asynchroneously open the wallet and emit a \
walletAsyncOpen signal when done + inline int openAsync(const QString& wallet, \
qlonglong wId, const QString& appid) + { return parent->openAsync(wallet, wId, \
appid); }
// Close and lock the wallet
// If force = true, will close it for all users. Behave. This
@@ -79,8 +80,8 @@
{ return parent->users(wallet); }
// Change the password of this wallet
- inline void changePassword(const QString& wallet, qlonglong wId, const QString& \
appid, const QDBusMessage& msg)
- { parent->changePassword(wallet, wId, appid, msg); }
+ inline void changePassword(const QString& wallet, qlonglong wId, const QString& \
appid) + { parent->changePassword(wallet, wId, appid); }
// A list of all wallets
inline QStringList wallets() const
@@ -174,6 +175,7 @@
void walletListDirty();
void walletCreated(const QString& wallet);
void walletOpened(const QString& wallet);
+ void walletAsyncOpened(int tId, int handle);
void walletDeleted(const QString& wallet);
void walletClosed(const QString& wallet);
void walletClosed(int handle);
Index: kdebase/runtime/kwalletd/CMakeLists.txt
===================================================================
--- kdebase/runtime/kwalletd/CMakeLists.txt (Revision 862863)
+++ kdebase/runtime/kwalletd/CMakeLists.txt (Arbeitskopie)
@@ -10,6 +10,7 @@
main.cpp
kbetterthankdialog.cpp
kwalletd.cpp
+ kwalletopenloop.cpp
kwalletwizard.cpp
ktimeout.cpp
kwalletsynctimer.cpp
["kwallet-kdelibs-asyncopen.diff" (text/x-patch)]
Index: kdelibs/kdeui/util/kwallet.h
===================================================================
--- kdelibs/kdeui/util/kwallet.h (Revision 862972)
+++ kdelibs/kdeui/util/kwallet.h (Arbeitskopie)
@@ -480,14 +480,16 @@
/**
* @internal
* Callback for kwalletd
+ * @param tId identifer for the open transaction
+ * @param handle the wallet's handle
*/
- void walletOpenResult(int rc);
+ void walletAsyncOpened(int tId, int handle);
/**
* @internal
* DBUS error slot.
*/
- void walletOpenError(const QDBusError& error);
+ void emitWalletAsyncOpenError();
/**
* @internal
Index: kdelibs/kdeui/util/kwallet.cpp
===================================================================
--- kdelibs/kdeui/util/kwallet.cpp (Revision 862972)
+++ kdelibs/kdeui/util/kwallet.cpp (Arbeitskopie)
@@ -112,6 +112,7 @@
QString name;
QString folder;
int handle;
+ int transactionId;
};
class KWalletDLauncher
@@ -204,41 +205,70 @@
Wallet *Wallet::openWallet(const QString& name, WId w, OpenType ot) {
if( w == 0 )
kWarning() << "Pass a valid window to KWallet::Wallet::openWallet().";
-
- QDBusMessage openMessage = QDBusMessage::createMethodCall(
- walletLauncher->getInterface().service(),
- walletLauncher->getInterface().path(),
- walletLauncher->getInterface().interface(),
- (ot == Path) ? "openPath" : "open");
- openMessage << name << qlonglong(w) << appid();
-
- if (ot == Asynchronous) {
- Wallet *wallet = new Wallet(-1, name);
- // place an asynchronous call
- walletLauncher->getInterface().connection().callWithCallback(openMessage, wallet,
- SLOT(walletOpenResult(int)), SLOT(walletOpenError(const QDBusError&)), 18000000);
+ Wallet *wallet = new Wallet(-1, name);
- return wallet;
+ // connect the daemon's opened signal to the slot filtering the
+ // signals we need
+ connect(&walletLauncher->getInterface(), SIGNAL(walletAsyncOpened(int, int)),
+ wallet, SLOT(walletAsyncOpened(int, int)));
+
+ // Use an eventloop for synchronous calls
+ QEventLoop loop;
+ if (ot == Synchronous) {
+ connect(wallet, SIGNAL(walletOpened(bool)), &loop, SLOT(quit()));
}
-
- // avoid deadlock if the app has some popup open (#65978/#71048)
- while( QWidget* widget = qApp->activePopupWidget())
- widget->close();
-
- QDBusMessage replyMessage = walletLauncher->getInterface().connection().call(
- openMessage, QDBus::Block, 18000000);
- if (replyMessage.type() == QDBusMessage::ReplyMessage) {
- QDBusReply<int> r(replyMessage);
- if (r.isValid()) {
- int drc = r;
- if (drc != -1) {
- return new Wallet(drc, name);
+
+ // do the call
+ QDBusReply<int> r;
+ if (ot == Synchronous || ot == Asynchronous) {
+ r = walletLauncher->getInterface().openAsync(name, (qlonglong)w, appid());
+ } else if (ot == Path) {
+ r = walletLauncher->getInterface().openPath(name, (qlonglong)w, appid());
+ } else {
+ delete wallet;
+ return 0;
+ }
+ wallet->d->transactionId = r.value();
+
+ switch (ot) {
+
+ case Synchronous:
+ // check for an immediate error
+ if (wallet->d->transactionId < 0) {
+ delete wallet;
+ wallet = 0;
+ } else {
+ // wait for the daemon's reply
+ loop.exec();
+ if (wallet->d->handle < 0) {
+ delete wallet;
+ return 0;
}
}
+ break;
+
+ case Asynchronous:
+ if (wallet->d->transactionId < 0) {
+ QTimer::singleShot(0, wallet, SLOT(emitWalletAsyncOpenError()));
+ // client code is responsible for deleting the wallet
+ }
+ break;
+
+ case Path:
+ if (wallet->d->transactionId < 0) {
+ delete wallet;
+ return 0;
+ } else {
+ wallet->d->handle = wallet->d->transactionId;
+ }
+ break;
+
+ default:
+ break;
}
- return 0;
+ return wallet;
}
@@ -683,26 +713,21 @@
}
}
-
-void Wallet::walletOpenResult(int id) {
- if (d->handle != -1) {
- // This is BAD.
+void Wallet::walletAsyncOpened(int tId, int handle) {
+ // ignore responses to calls other than ours
+ if (d->transactionId != tId || d->handle != -1) {
return;
}
-
- if (id > 0) {
- d->handle = id;
- emit walletOpened(true);
- } else if (id < 0) {
- emit walletOpened(false);
- } // id == 0 => wait
+
+ // disconnect the async signal
+ disconnect(this, SLOT(walletAsyncOpened(int, int)));
+
+ d->handle = handle;
+ emit walletOpened(handle > 0);
}
-void Wallet::walletOpenError(const QDBusError& error)
-{
- if (error.isValid()) {
- emit walletOpened(false);
- }
+void Wallet::emitWalletAsyncOpenError() {
+ emit walletOpened(false);
}
bool Wallet::folderDoesNotExist(const QString& wallet, const QString& folder)
Index: kdelibs/kdeui/util/org.kde.KWallet.xml
===================================================================
--- kdelibs/kdeui/util/org.kde.KWallet.xml (Revision 862972)
+++ kdelibs/kdeui/util/org.kde.KWallet.xml (Arbeitskopie)
@@ -9,6 +9,10 @@
<signal name="walletOpened">
<arg name="wallet" type="s" direction="out"/>
</signal>
+ <signal name="walletAsyncOpened">
+ <arg name="tId" type="i" direction="out"/>
+ <arg name="handle" type="i" direction="out"/>
+ </signal>
<signal name="walletDeleted">
<arg name="wallet" type="s" direction="out"/>
</signal>
@@ -46,6 +50,12 @@
<arg name="wId" type="x" direction="in"/>
<arg name="appid" type="s" direction="in"/>
</method>
+ <method name="openAsync">
+ <arg type="i" direction="out"/>
+ <arg name="wallet" type="s" direction="in"/>
+ <arg name="wId" type="x" direction="in"/>
+ <arg name="appid" type="s" direction="in"/>
+ </method>
<method name="close">
<arg type="i" direction="out"/>
<arg name="wallet" type="s" direction="in"/>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic