| Summary: | VNC plugin deadlocks on connection | ||
|---|---|---|---|
| Product: | [Applications] krdc | Reporter: | Thiago Macieira <thiago> |
| Component: | VNC | Assignee: | Urs Wolfer <uwolfer> |
| Status: | RESOLVED FIXED | ||
| Severity: | grave | CC: | chemobejk, nicolas.fella, vasyl.demin |
| Priority: | NOR | ||
| Version First Reported In: | 24.02.2 | ||
| Target Milestone: | --- | ||
| Platform: | Other | ||
| OS: | Linux | ||
| Latest Commit: | https://invent.kde.org/network/krdc/-/commit/e6a169e40b80b4fcd5e42dbba17d82f3f7bf4feb | Version Fixed In: | |
| Sentry Crash Report: | |||
I'd say "vncplugin doesn't work at all, ever, under any conditions, if you try to use it" is sufficient for "critical", but ok for "grave" BTW, I tried to fix that function myself. I can apply a workaround to just restore the previous behaviour, but that's papering over the sun with a sieve. I really don't understand what's going on and what is being locked. Is it trying to protect m_stopped? Why not use an atomic instead? Or, better, QThread::isInterruptionRequested()? But on line 549, it accesses m_stopped without a mutex, so I don't know if it needed protecting in the first place. Ditto for m_passwordError: why not an atomic? Or why not use a local variable and only lock to save the state when exiting the function? This code should be rewritten with closer locks, not that one QMutexLocker at the top that is used incorrectly. A possibly relevant merge request was started @ https://invent.kde.org/network/krdc/-/merge_requests/99 Git commit 48fbcdffc4ff22a11b56d8809739e0f096d190e6 by Nicolas Fella, on behalf of Thiago Macieira. Committed on 30/04/2024 at 22:45. Pushed by nicolasfella into branch 'release/24.02'. VNC: replace m_stopped with QThread's interruptionRequested It's an out-of-line function call, but it doesn't require a mutex for most uses. The uses in the callbacks are replaced with isRunning(), because interruptionRequested resets back to false as soon as run() exits. (cherry picked from commit 33f89ef0bb4f5186dad7ceecad599a0e834e7f88) M +14 -24 vnc/vncclientthread.cpp M +0 -1 vnc/vncclientthread.h https://invent.kde.org/network/krdc/-/commit/48fbcdffc4ff22a11b56d8809739e0f096d190e6 *** Bug 485215 has been marked as a duplicate of this bug. *** I can confirm that after applying the patch VNC connections work again with krdc. Git commit e6a169e40b80b4fcd5e42dbba17d82f3f7bf4feb by Nicolas Fella, on behalf of Thiago Macieira. Committed on 08/06/2024 at 18:42. Pushed by nicolasfella into branch 'release/24.05'. VNC: replace m_stopped with QThread's interruptionRequested It's an out-of-line function call, but it doesn't require a mutex for most uses. The uses in the callbacks are replaced with isRunning(), because interruptionRequested resets back to false as soon as run() exits. (cherry picked from commit 33f89ef0bb4f5186dad7ceecad599a0e834e7f88) M +14 -24 vnc/vncclientthread.cpp M +0 -1 vnc/vncclientthread.h https://invent.kde.org/network/krdc/-/commit/e6a169e40b80b4fcd5e42dbba17d82f3f7bf4feb |
SUMMARY VNC plugin deadlocks as soon as you try to connect STEPS TO REPRODUCE 1. Launch KRDC 2. Attempt to connect to a VNC host 3. Close KRDC OBSERVED RESULT KRDC freezes and must be killed EXPECTED RESULT KRDC closes. Preferably, the connection does establish. SOFTWARE/OS VERSIONS KDE Plasma Version: 6.0.3 KDE Frameworks Version: 6.1.0 Qt Version: 6.7.0 ADDITIONAL INFORMATION Upon trying to connect, KRDC is still usable, but the debugger shows one thread to be already frozen: Thread 30 (Thread 0x7fffb4a006c0 (LWP 497638) "VncClientThread"): #0 0x00007ffff5711bcd in syscall () at /lib64/libc.so.6 #1 0x00007ffff60dcca5 in QBasicMutex::lockInternal() () at /lib64/glibc-hwcaps/x86-64-v4/libQt6Core.so.6.7.0 #2 0x00007fffe341886c in QBasicMutex::lock() (this=0x555555b37208) at /usr/include/qt6/QtCore/qmutex.h:41 #3 QMutexLocker<QMutex>::relock() (this=<synthetic pointer>) at /usr/include/qt6/QtCore/qmutex.h:257 #4 VncClientThread::run() (this=0x555555b37180) at /usr/src/debug/krdc-24.02.2/vnc/vncclientthread.cpp:521 #5 0x00007ffff60dc1b8 in () at /lib64/glibc-hwcaps/x86-64-v4/libQt6Core.so.6.7.0 #6 0x00007ffff5692bb2 in start_thread () at /lib64/libc.so.6 #7 0x00007ffff571400c in clone3 () at /lib64/libc.so.6 QMutexLocker locker(&mutex); while (!m_stopped) { // try to connect as long as the server allows locker.relock(); m_passwordError = false; This will NEVER EVER work because the mutex is locked by that locker and nothing can unlock it. This is a difference in behaviour between Qt 5 and Qt 6. Qt 5: https://codebrowser.dev/qt5/qtbase/src/corelib/thread/qmutex.h.html#_ZN12QMutexLocker6relockEv Qt 6: https://codebrowser.dev/qt6/qtbase/src/corelib/thread/qmutex.h.html#_ZN12QMutexLocker6relockEv The change happened in Qt 6.4, commit 1b1456975347b044c11169458b53c9f6083dbc59