Re: Review Request 127121: KWallet::openWallet(Synchronous) : time out after 1 hour rather than 25 seconds.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127121/ --- (Updated Feb. 25, 2016, 12:16 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Valentin Rusu. Changes --- Submitted with commit 40f1fd03fc6d86f206322a100c5e1ead0df33493 by David Faure to branch master. Repository: kwallet Description --- The default DBus timeout is 25 seconds, which means that if the user went to get a cup of tea during session startup, when they come back they get prompted with all sorts of additional non-kwallet password requests due to all kwallet requests having timed out. I added setTimeout in QDBusAbstractInterface in Qt 4.8 for things like this. Testcase: eval `dbus-launch` export SSH_ASKPASS=$KDEDIR/bin/ksshaskpass ssh-add < /dev/null (but the same happens with the IMAP resource etc.) Diffs - src/api/KWallet/kwallet.cpp b72edad19840943f70755c8668668a1881f1fb39 Diff: https://git.reviewboard.kde.org/r/127121/diff/ Testing --- (see commit log) Thanks, David Faure ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127121: KWallet::openWallet(Synchronous) : time out after 1 hour rather than 25 seconds.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127121/#review92667 --- Ship it! Ship It! - David Edmundson On Feb. 20, 2016, 8:48 p.m., David Faure wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127121/ > --- > > (Updated Feb. 20, 2016, 8:48 p.m.) > > > Review request for KDE Frameworks and Valentin Rusu. > > > Repository: kwallet > > > Description > --- > > The default DBus timeout is 25 seconds, which means that if the user went to > get a cup of tea during session startup, when they come back they get prompted > with all sorts of additional non-kwallet password requests due to all kwallet > requests having timed out. > > I added setTimeout in QDBusAbstractInterface in Qt 4.8 for things like this. > > Testcase: > eval `dbus-launch` > export SSH_ASKPASS=$KDEDIR/bin/ksshaskpass > ssh-add < /dev/null > (but the same happens with the IMAP resource etc.) > > > Diffs > - > > src/api/KWallet/kwallet.cpp b72edad19840943f70755c8668668a1881f1fb39 > > Diff: https://git.reviewboard.kde.org/r/127121/diff/ > > > Testing > --- > > (see commit log) > > > Thanks, > > David Faure > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127121: KWallet::openWallet(Synchronous) : time out after 1 hour rather than 25 seconds.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127121/ --- (Updated Feb. 20, 2016, 8:48 p.m.) Review request for KDE Frameworks and Valentin Rusu. Changes --- Make it 2<<30 ms, i.e. 12 days if I counted correctly. I don't dare making it bigger, but it should be plenty ;) Repository: kwallet Description --- The default DBus timeout is 25 seconds, which means that if the user went to get a cup of tea during session startup, when they come back they get prompted with all sorts of additional non-kwallet password requests due to all kwallet requests having timed out. I added setTimeout in QDBusAbstractInterface in Qt 4.8 for things like this. Testcase: eval `dbus-launch` export SSH_ASKPASS=$KDEDIR/bin/ksshaskpass ssh-add < /dev/null (but the same happens with the IMAP resource etc.) Diffs (updated) - src/api/KWallet/kwallet.cpp b72edad19840943f70755c8668668a1881f1fb39 Diff: https://git.reviewboard.kde.org/r/127121/diff/ Testing --- (see commit log) Thanks, David Faure ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127121: KWallet::openWallet(Synchronous) : time out after 1 hour rather than 25 seconds.
> On Feb. 20, 2016, 8:25 p.m., Andreas Hartmetz wrote: > > Sorry for being the guy who says "I don't like this improvement because > > it's still not perfect"... > > Why have a timeout at all and not, for example, retry after getting a > > timeout error? (A long timeout still makes sense to avoid most UI glitches) > > I hope nobody uses the synchronous API in the UI thread, it would be a > > terrible idea to do that anyway. Is this for e.g. Akonadi agents? Those > > could as well wait indefinitely. If they can't -> well they shouldn't use > > synchronous API. Synchronous IPC to an unreliable peer (if the user is > > involved it's unreliable ;) is asking for problems. > > David Faure wrote: > Hi Andreas, thanks for your input. (I would also welcome your input on > https://git.reviewboard.kde.org/r/127077/ even though I just pushed it). > > The use cases I saw (where the timeout was a problem with a sync call) > are ksshaskpass (no GUI yet at that point) and the imap resource. So yes, > infinite wait would be good, I agree. I just don't know how big a number DBus > supports for a timeout. Shall we make it 2^31-1 to avoid any signedness > issues? I'm not going to actually test if it can wait that long though ;) Some very large number would be fine. If something has a problem with an extremely long timeout it will have the same problem with an hour-long timeout. And if that something has a problem with it, that is its own fault. Sorry about the other one, I noticed it too late. I can still have a look. - Andreas --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127121/#review92589 --- On Feb. 20, 2016, 2:16 p.m., David Faure wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127121/ > --- > > (Updated Feb. 20, 2016, 2:16 p.m.) > > > Review request for KDE Frameworks and Valentin Rusu. > > > Repository: kwallet > > > Description > --- > > The default DBus timeout is 25 seconds, which means that if the user went to > get a cup of tea during session startup, when they come back they get prompted > with all sorts of additional non-kwallet password requests due to all kwallet > requests having timed out. > > I added setTimeout in QDBusAbstractInterface in Qt 4.8 for things like this. > > Testcase: > eval `dbus-launch` > export SSH_ASKPASS=$KDEDIR/bin/ksshaskpass > ssh-add < /dev/null > (but the same happens with the IMAP resource etc.) > > > Diffs > - > > src/api/KWallet/kwallet.cpp b72edad19840943f70755c8668668a1881f1fb39 > > Diff: https://git.reviewboard.kde.org/r/127121/diff/ > > > Testing > --- > > (see commit log) > > > Thanks, > > David Faure > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127121: KWallet::openWallet(Synchronous) : time out after 1 hour rather than 25 seconds.
> On Feb. 20, 2016, 8:25 p.m., Andreas Hartmetz wrote: > > Sorry for being the guy who says "I don't like this improvement because > > it's still not perfect"... > > Why have a timeout at all and not, for example, retry after getting a > > timeout error? (A long timeout still makes sense to avoid most UI glitches) > > I hope nobody uses the synchronous API in the UI thread, it would be a > > terrible idea to do that anyway. Is this for e.g. Akonadi agents? Those > > could as well wait indefinitely. If they can't -> well they shouldn't use > > synchronous API. Synchronous IPC to an unreliable peer (if the user is > > involved it's unreliable ;) is asking for problems. Hi Andreas, thanks for your input. (I would also welcome your input on https://git.reviewboard.kde.org/r/127077/ even though I just pushed it). The use cases I saw (where the timeout was a problem with a sync call) are ksshaskpass (no GUI yet at that point) and the imap resource. So yes, infinite wait would be good, I agree. I just don't know how big a number DBus supports for a timeout. Shall we make it 2^31-1 to avoid any signedness issues? I'm not going to actually test if it can wait that long though ;) - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127121/#review92589 --- On Feb. 20, 2016, 2:16 p.m., David Faure wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127121/ > --- > > (Updated Feb. 20, 2016, 2:16 p.m.) > > > Review request for KDE Frameworks and Valentin Rusu. > > > Repository: kwallet > > > Description > --- > > The default DBus timeout is 25 seconds, which means that if the user went to > get a cup of tea during session startup, when they come back they get prompted > with all sorts of additional non-kwallet password requests due to all kwallet > requests having timed out. > > I added setTimeout in QDBusAbstractInterface in Qt 4.8 for things like this. > > Testcase: > eval `dbus-launch` > export SSH_ASKPASS=$KDEDIR/bin/ksshaskpass > ssh-add < /dev/null > (but the same happens with the IMAP resource etc.) > > > Diffs > - > > src/api/KWallet/kwallet.cpp b72edad19840943f70755c8668668a1881f1fb39 > > Diff: https://git.reviewboard.kde.org/r/127121/diff/ > > > Testing > --- > > (see commit log) > > > Thanks, > > David Faure > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127121: KWallet::openWallet(Synchronous) : time out after 1 hour rather than 25 seconds.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127121/#review92589 --- Sorry for being the guy who says "I don't like this improvement because it's still not perfect"... Why have a timeout at all and not, for example, retry after getting a timeout error? (A long timeout still makes sense to avoid most UI glitches) I hope nobody uses the synchronous API in the UI thread, it would be a terrible idea to do that anyway. Is this for e.g. Akonadi agents? Those could as well wait indefinitely. If they can't -> well they shouldn't use synchronous API. Synchronous IPC to an unreliable peer (if the user is involved it's unreliable ;) is asking for problems. - Andreas Hartmetz On Feb. 20, 2016, 2:16 p.m., David Faure wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127121/ > --- > > (Updated Feb. 20, 2016, 2:16 p.m.) > > > Review request for KDE Frameworks and Valentin Rusu. > > > Repository: kwallet > > > Description > --- > > The default DBus timeout is 25 seconds, which means that if the user went to > get a cup of tea during session startup, when they come back they get prompted > with all sorts of additional non-kwallet password requests due to all kwallet > requests having timed out. > > I added setTimeout in QDBusAbstractInterface in Qt 4.8 for things like this. > > Testcase: > eval `dbus-launch` > export SSH_ASKPASS=$KDEDIR/bin/ksshaskpass > ssh-add < /dev/null > (but the same happens with the IMAP resource etc.) > > > Diffs > - > > src/api/KWallet/kwallet.cpp b72edad19840943f70755c8668668a1881f1fb39 > > Diff: https://git.reviewboard.kde.org/r/127121/diff/ > > > Testing > --- > > (see commit log) > > > Thanks, > > David Faure > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127121: KWallet::openWallet(Synchronous) : time out after 1 hour rather than 25 seconds.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127121/ --- (Updated Feb. 20, 2016, 2:16 p.m.) Review request for KDE Frameworks and Valentin Rusu. Changes --- Only change timeout around the sync wallet open call, leave 25s for the other calls, just in case. Repository: kwallet Description --- The default DBus timeout is 25 seconds, which means that if the user went to get a cup of tea during session startup, when they come back they get prompted with all sorts of additional non-kwallet password requests due to all kwallet requests having timed out. I added setTimeout in QDBusAbstractInterface in Qt 4.8 for things like this. Testcase: eval `dbus-launch` export SSH_ASKPASS=$KDEDIR/bin/ksshaskpass ssh-add < /dev/null (but the same happens with the IMAP resource etc.) Diffs (updated) - src/api/KWallet/kwallet.cpp b72edad19840943f70755c8668668a1881f1fb39 Diff: https://git.reviewboard.kde.org/r/127121/diff/ Testing --- (see commit log) Thanks, David Faure ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 127121: KWallet::openWallet(Synchronous) : time out after 1 hour rather than 25 seconds.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127121/ --- Review request for KDE Frameworks and Valentin Rusu. Repository: kwallet Description --- The default DBus timeout is 25 seconds, which means that if the user went to get a cup of tea during session startup, when they come back they get prompted with all sorts of additional non-kwallet password requests due to all kwallet requests having timed out. I added setTimeout in QDBusAbstractInterface in Qt 4.8 for things like this. Testcase: eval `dbus-launch` export SSH_ASKPASS=$KDEDIR/bin/ksshaskpass ssh-add < /dev/null (but the same happens with the IMAP resource etc.) Diffs - src/api/KWallet/kwallet.cpp b72edad19840943f70755c8668668a1881f1fb39 Diff: https://git.reviewboard.kde.org/r/127121/diff/ Testing --- (see commit log) Thanks, David Faure ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
