https://bugs.kde.org/show_bug.cgi?id=517659
Simon Redman <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Latest Commit| |https://invent.kde.org/netw | |ork/kdeconnect-kde/-/commit | |/3bf16922bdef2c29be15d8b2ee | |3bdcbb43e710f1 Status|ASSIGNED |RESOLVED --- Comment #3 from Simon Redman <[email protected]> --- Git commit 3bf16922bdef2c29be15d8b2ee3bdcbb43e710f1 by Simon Redman, on behalf of Bill Sterzenbach. Committed on 05/05/2026 at 00:46. Pushed by sredman into branch 'master'. Fix silent send failure on SMS conversation cache miss ## Summary When the daemon's SMS conversation cache does not contain the target thread (e.g. after a device reconnect, or before the SMS app has been opened for this session), `replyToConversation()` used to log a warning and silently drop the user's message. This MR makes the daemon request the missing conversation from the phone, wait for it, and then actually send the queued message on the same call. ## Test Plan ### Setup - KDE Connect paired with Android phone (SMS plugin enabled on Android). - Desktop daemon (`kdeconnectd`) and `kdeconnect-sms`. ### Cache-hit (happy path) 1. Open SMS app, let conversation list populate. 2. Send a message in any existing conversation. 3. Verify the message is delivered. ### Cache-miss (the fix) 1. Kill the daemon (`killall kdeconnectd`). 2. Launch both daemon and `kdeconnect-sms` in quick succession. 3. Attempt to send a message to a known conversation before the conversation list finishes loading. 4. Verify the message is delivered (with a small delay while the phone responds). ### Concurrent wait (QReadWriteLock refactor) 1. Open a conversation and scroll up rapidly to trigger `RequestConversationWorker::updateConversation()`. 2. While messages are still loading, send a new message. 3. Both threads call `updateConversation()` on the same ID; verify no deadlock, crash, or dropped send. ## Implementation Notes ### Why a worker thread on cache miss? `updateConversation()` blocks until the phone's reply arrives via `addMessages()`. `addMessages()` is delivered on the main thread, so blocking `replyToConversation()` (also called on the main thread) would deadlock. The worker thread pattern preserves main-thread responsiveness. ### Why QThread::create instead of QObject + moveToThread? The existing `RequestConversationWorker` uses `QObject + moveToThread` because it needs to emit signals back to the main thread. Our worker is one-shot and doesn't need to communicate back, so `QThread::create` is sufficient and avoids introducing another header + cpp pair. ### Why QReadWriteLock? The old `updateConversation()` logic refused to let more than one thread wait for the same conversation (it returned early). With the new cache-miss worker, a second thread could reasonably want to wait alongside an in-flight request (for example, the user triggers a scroll-to-load-more while the first-send worker is still waiting). Switching to `QReadWriteLock` lets multiple threads share the wait on a read lock while still serialising the request-initiation under a write lock. ### Known limitations - If the phone never responds (disconnected, doesn't have the conversation), the worker thread blocks until daemon shutdown. Same pre-existing behavior as `RequestConversationWorker`. Could be addressed with a timeout in a follow-up. - A retry loop (`maxRetries = 3`) guards against BUG 400488 style empty responses from the phone. ## Disclosure This MR was developed with AI coding assistant support (Windsurf / Claude). All code was reviewed by me, tested end-to-end on Linux against an Android device, and iterated based on review feedback from @sredman in the previous revision. M +61 -13 plugins/sms/conversationsdbusinterface.cpp M +9 -1 plugins/sms/conversationsdbusinterface.h https://invent.kde.org/network/kdeconnect-kde/-/commit/3bf16922bdef2c29be15d8b2ee3bdcbb43e710f1 -- You are receiving this mail because: You are watching all bug changes.
