Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

delay / hang on exit #6101

Closed
mx1up opened this issue Feb 10, 2021 · 18 comments
Closed

delay / hang on exit #6101

mx1up opened this issue Feb 10, 2021 · 18 comments

Comments

@mx1up
Copy link

mx1up commented Feb 10, 2021

Overview

Since some time, KeepassXC hangs for a few seconds after exiting. It only happens when a password has been copied to the clipboard, otherwise it exits immediately. It is not really keepassxc itself that hangs, but rather the desktop interface: keepass remains visible in the task bar, i cannot click on other items in the taskbar, cannot select items on desktop. Funnily enough, if i have vivaldi browser open, i can still scroll click around there. Maybe it is a dbus related problem..?

Steps to Reproduce

  1. open database
  2. copy password
  3. exit

Expected Behavior

application exits immediately, does not block desktop

Actual Behavior

application exits, but stays in task bar and blocks desktop

Context

I am on opensuse tumbleweed, but could also verify this behavior on opensuse leap 15.1 running keepassxc 2.6.2
I made a screencast showcasing the problem: https://youtu.be/JUjCBmVcyig

KeePassXC - Version 2.6.4
Revision: 34a78f0

Qt 5.15.2
Debugging mode is disabled.

Operating system: openSUSE Tumbleweed
CPU architecture: x86_64
Kernel: linux 5.10.12-1-default

Enabled extensions:

  • Auto-Type
  • Browser Integration
  • SSH Agent
  • KeeShare (only unsigned sharing)
  • YubiKey
  • Secret Service Integration

Cryptographic libraries:

  • libgcrypt 1.9.1-unknown

Operating System: Linux
Desktop Env: KDE
Windowing System: X11

@mx1up mx1up added the bug label Feb 10, 2021
@droidmonkey
Copy link
Member

I'm not sure how this could be a keepassxc caused issue to be honest.

@mx1up
Copy link
Author

mx1up commented Feb 11, 2021

i did see the keepassxc command return immediately in the console, so I was hoping maybe something dbus related.. ;) or clearing the clipboard (but I already tried with that feature turned off). I also checked that all plugins are turned off.

any ideas on how to debug this or validate it's not keepassxc..?

@droidmonkey
Copy link
Member

If the command returned from the console then the process is no longer running. Use top or activity monitor to confirm its not running.

@mx1up
Copy link
Author

mx1up commented Feb 11, 2021

the desktop recording already predicted it, but yes, i validated it by running
keepassxc && pgrep keepassxc
the "release" happens after 5 seconds. I guess it must be some dbus related timeout.
I will create a bug report upstream.

It would be nice to know however, if other users are experiencing the same problem. I have not encountered another application yet that provokes the same behavior. That's why I suspect something like using the "hide password" clipboard integration or clearing the clipboard (albeit a bug on the other side then).

@droidmonkey
Copy link
Member

DBus has nothing to do with the clipboard. We don't have any hide password clipboard integration or hooks to clear the clipboard. If the app is still running the clipboard is cleared by sending a blank text string to the clipboard after the set delay.

@mx1up
Copy link
Author

mx1up commented Feb 12, 2021

@droidmonkey about hiding the password, I'm not sure how you call it, but it is hidden by this PR:
https://github.com/keepassxreboot/keepassxc/pull/1969/files

dbus: it was just a hunch, now i know keepassxc does not use dbus or any other process, i guess. i see dbus messages passing for the task bar when an item is added/removed.

@droidmonkey
Copy link
Member

That just tells klipper not to store it in history. It is all part of the clipboard process, no special function calls or anything.

@Hasshu
Copy link

Hasshu commented May 11, 2021

@mx1up I believe I've been encountering this exact issue for quite some time (KDE Plasma as well). Assuming you have created an upstream bug report, could you post a link to it?

@mx1up
Copy link
Author

mx1up commented May 11, 2021

@Hasshu correct, unfortunately no progress so far :/ https://bugs.kde.org/show_bug.cgi?id=432810

@Hasshu
Copy link

Hasshu commented Dec 8, 2021

Alright, I think I'm onto something... I decided to give the latest AppImage a try, and it seems to be freezing Plasma even more often than the version from the repo. But there's another thing I noticed: it freezes at least some of the running Qt-based software as well (Dolphin, Gwenview, and Plasma's System Settings, to name a few). Oddly enough, applications like GNOME Disks (GTK) and dnfdragora (libYUI) appear to be unaffected.

That often can be reproduced by changing a setting that requires a restart of KeePassXC (e.g., by toggling the compact mode), then restarting it.

@mx1up
Copy link
Author

mx1up commented Jan 9, 2023

hi @droidmonkey , @phoerious

I did some further investigation: I checked out older versions until I was at the last working version: v2.5.2. The problematic commit in v2.5.3 is 6f9907a.

When I use the clear method ( b8268bb ) it works again as expected. Of course, due to #4126, it seems the whole point was to avoid the clear() method :) Fortunately, in the mean time, according to @rmader gnome has now better support for this use case: #4126 (comment)

I don't know if there are any other side effects, but I would like to propose to reinstate the use of the clear() method (I can make a PR if you agree).

thanks for considering

btw, I also tried out several Qt 5.x versions, up until the latest I could freely find binaries available for (5.15.2). It did not make any difference.

@droidmonkey
Copy link
Member

I'm not following, what works again with clear()?

@mx1up
Copy link
Author

mx1up commented Jan 9, 2023

when using clear instead of setting empty text, the task bar and desktop does not longer hang after exiting keepassxc and having copied a password to the clipboard (see initial description of this issue for more details/video).

@mx1up
Copy link
Author

mx1up commented Jan 9, 2023

btw, I forgot to mention I also was able to reproduce the bug on KDE Neon distribution (ubuntu based), which had still a more recent Qt version:

Operating System: KDE neon Unstable Edition
KDE Plasma Version: 5.26.80
KDE Frameworks Version: 5.102.0
Qt Version: 5.15.7
Kernel Version: 5.15.0-57-generic (64-bit)
Graphics Platform: X11
Processors: 6 × Intel® Core™ i7-9850H CPU @ 2.60GHz
Memory: 11,7 GiB of RAM
Graphics Processor: llvmpipe
Manufacturer: innotek GmbH
Product Name: VirtualBox
System Version: 1.2

@droidmonkey
Copy link
Member

droidmonkey commented Jan 9, 2023

If clear() still doesn't actually clear the clipboard on X11 then no, we won't revert to that code. The fix is to update your gnome to that fixed version (now 2 years old) and tell kde to fix it as well. There is nothing inherently wrong with setting a blank clipboard entry, that is not OUR bug.

@mx1up
Copy link
Author

mx1up commented Jan 9, 2023

  • clear() does clear the clipboard on X11, but I could only test on KDE (and gnome reportedly fixed their bug)
  • the gnome and kde bugs are 2 unrelated bugs (in gnome the clipboard was not correctly cleared, on kde, desktop hangs)
  • I agree that there is nothing inherently wrong by setting a blank clipboard entry and that it is not your bug. Then again, there is also nothing inherently wrong with clearing the clipboard, which is the more logical approach (hence the original code). By your reasoning, you should not have updated the keepassxc code to circumvent the gnome bug in the first place and just told gnome to fix their bug? :) (which they did). That's why I thought it was worth the shot to ask to adapt the keepassxc code back to the "normal" code.

note: clearing the clipboard using my modified version b8268bb , reset the clipboard to what was on it before the password was copied, which is actually a benefit imho (like you never copied a password).

@droidmonkey
Copy link
Member

Touche, but we fixed a security problem, so that's important 😁

If clear() works everywhere, we go back to it, no problems. I looked at the code for xcb clipboard clearing on qt side and it just sets the mime data to null which is close to what we are doing.

@michaelk83
Copy link

The intermediate solution is to set an empty text, and then call clear(). If clear() fails, you are still left with an empty text instead of the password; if clear() works, the empty text is removed and the hang is avoided.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants