Fixing kdeconnect-sms

I received a Google Drive link by SMS with a few family photos. Normally, I would use KDE's handy phone integration, KDE Connect, to open the message on my desktop. Unfortunately, clicking on this conversation - and only this conversation, would cause kdeconnect-sms to crash. Poof. We could just get the link some other way, but the fun of open source is being able to just fix the things that bug you.

What's happening?

As a user all we saw was the application closing. We get no notification, nothing in KDE's error reporting daemon, nada. So lets try the simple thing first and open up kdeconnect-sms in a terminal to see if we get any useful output:

❯ kdeconnect-sms
qrc:/qml/ConversationList.qml:51: TypeError: Cannot read property 'text' of null
qrc:/qml/ConversationList.qml:267: TypeError: Cannot read property 'text' of null
file:///usr/lib/qt/qml/org/kde/kirigami.2/templates/InlineMessage.qml:261:9: QML ActionToolBar: Binding loop detected for property "atBottom"
ktp-people: Account manager ready
qrc:/qml/ChatMessage.qml:109: TypeError: Cannot read property 'length' of undefined
qrc:/qml/SendingArea.qml:69:17: QML TextArea: Possible anchor loop detected on fill.
zsh: segmentation fault (core dumped)  kdeconnect-sms

Ahah, it's a segmentation fault! Since I'm using Arch Linux, which uses systemd, we can use coredumpctl to inspect the core dump:

❯ coredumpctl debug -1
...

The -1 tells coredumpctl to open the last core dump recorded, and debug tells coredumpctl to open it in a debugger which in this case will be gdb. Once it's loaded we can use bt to display a backtrace:

[Current thread is 1 (Thread 0x7efbfe5ffcc0 (LWP 66022))]
(gdb) bt
#0  0x000055a24e3abf95 in ?? ()
#1  0x000055a24e3acd65 in ?? ()
#2  0x00007fe9cfc467a0 in ?? () from /usr/lib/libQt5Core.so.5
#3  0x00007fe9d1874121 in ?? () from /usr/lib/libkdeconnectinterfaces.so.21
#4  0x00007fe9d1874683 in ?? () from /usr/lib/libkdeconnectinterfaces.so.21
#5  0x00007fe9d0c61300 in ?? () from /usr/lib/libQt5DBus.so.5
#6  0x00007fe9cfc3c50f in QObject::event(QEvent*) () from /usr/lib/libQt5Core.so.5
#7  0x00007fe9d06e4d62 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/lib/libQt5Widgets.so.5
#8  0x00007fe9cfc0f3ba in QCoreApplication::notifyInternal2(QObject*, QEvent*) () from /usr/lib/libQt5Core.so.5
#9  0x00007fe9cfc124b9 in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) () from /usr/lib/libQt5Core.so.5
#10 0x00007fe9cfc689b4 in ?? () from /usr/lib/libQt5Core.so.5
#11 0x00007fe9ce47610c in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.0
#12 0x00007fe9ce4c9ba9 in ?? () from /usr/lib/libglib-2.0.so.0
#13 0x00007fe9ce473871 in g_main_context_iteration () from /usr/lib/libglib-2.0.so.0
#14 0x00007fe9cfc67fe6 in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/libQt5Core.so.5
#15 0x00007fe9cfc0dd2c in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/libQt5Core.so.5
#16 0x00007fe9cfc16294 in QCoreApplication::exec() () from /usr/lib/libQt5Core.so.5
#17 0x000055a24e3a015d in ?? ()
#18 0x00007fe9cf599b25 in __libc_start_main () from /usr/lib/libc.so.6
#19 0x000055a24e3a089e in _start ()

This doesn't give us a lot of information. Since kdeconnect-sms wasn't built for debugging, it doesn't including the debug symbols that would fill in the ?? for us.

Filling in the blanks

When you're trying to debug a binary, having the debugging symbols can be the difference between a 10-minute exercise or a 3-hour expedition through assembly with a debugger. Thankfully, kdeconnect-sms is open source, so we can simply build our own version for debugging. We'll get the source from KDE's gitlab:

❯ git clone https://invent.kde.org/network/kdeconnect-kde.git
❯ cd kdeconnect-kde

There's no mention of it in the README, but you'll also need KDE's ECM, or extra-cmake-modules, in order to build kdeconnect. Since I'm on Arch, this is simply:

❯ sudo pacman -S extra-cmake-modules

And if you don't have it yet, get cmake as well:

❯ sudo pacman -S cmake

Now we'll configure our copy of kdeconnect with debugging symbols, as well as changing the installation location, so it doesn't mess with the system-wide kdeconnect:

❯ mkdir build
❯ cd build
❯ cmake -DCMAKE_INSTALL_PREFIX=~/kde-prefix -DCMAKE_BUILD_TYPE=Debug ..

And finally we actually build and install it:

make -j$(nproc) install

The -j$(nproc) tells make to build in parallel with as many threads as possible, which can speed up this initial compile significantly. Unless you've got a beefy machine, go grab a coffee.

Once it's done compiling, you might be tempted to just run it, but you're more likely to get a crash or a blank screen than anything useful. KDE applications are complex beasts, and KDE applications sometimes need a bunch of scaffolding. Thankfully the build step created a file for you, called prefix.sh in the build directory to make this easy. We also want to kill the background daemon from the global kdeconnect install if it's running, since it may be running a very different version than what we're working on:

❯ killall kdeconnectd kdeconnect-sms
❯ source prefix.sh

Now we start our version of the daemon (and put it in the background with &), which may be in lib or lib64, depending on your environment:

❯ ~/kde-prefix/lib64/libexec/kdeconnectd &

And, finally, we start our version of kdeconnect-sms. If you're doing this in a new shell, remember to source prefix.sh before running it:

❯ kdeconnect-sms

Now when kdeconnect-sms crashes, we can go back and run coredumpctl debug -1 again, but this time we get a very useful backtrace:

[Current thread is 1 (Thread 0x7efbfe5ffcc0 (LWP 66022))]
(gdb) bt
#0  0x00007f343ccb1d22 in raise () from /usr/lib/libc.so.6
#1  0x00007f343b6fb3e0 in KCrash::defaultCrashHandler(int) () from /usr/lib/libKF5Crash.so.5
#2  <signal handler called>
#3  0x00007f343ccb1d22 in raise () from /usr/lib/libc.so.6
#4  0x00007f343cc9b862 in abort () from /usr/lib/libc.so.6
#5  0x00007f343d248910 in QMessageLogger::fatal(char const*, ...) const () from /usr/lib/libQt5Core.so.5
#6  0x00007f343d247cf5 in qt_assert(char const*, char const*, int) () from /usr/lib/libQt5Core.so.5
#7  0x0000561602056c94 in QList<ConversationAddress>::first (this=0x7fff6e344410) at /usr/include/qt/QtCore/qlist.h:361
#8  0x00005616020551d1 in ConversationModel::createRowFromMessage (this=0x561603a894e0, message=..., pos=0) at /home/tktech/projects/kdeconnect-kde/smsapp/conversationmodel.cpp:145
#9  0x0000561602055985 in ConversationModel::handleConversationUpdate (this=0x561603a894e0, msg=...) at /home/tktech/projects/kdeconnect-kde/smsapp/conversationmodel.cpp:189
#10 0x0000561602041ac6 in ConversationModel::qt_static_metacall (_o=0x561603a894e0, _c=QMetaObject::InvokeMetaMethod, _id=2, _a=0x7fff6e344690)
    at /home/tktech/projects/kdeconnect-kde/build/smsapp/kdeconnect-sms_autogen/EWIEGA46WW/moc_conversationmodel.cpp:161
#11 0x00007f343d4a87a0 in ?? () from /usr/lib/libQt5Core.so.5
#12 0x00007f343f73baf0 in OrgKdeKdeconnectDeviceConversationsInterface::conversationUpdated (this=0x561603e78640, _t1=...)
    at /home/tktech/projects/kdeconnect-kde/build/interfaces/conversationsinterface.moc:265
#13 0x00007f343f73b2dd in OrgKdeKdeconnectDeviceConversationsInterface::qt_static_metacall (_o=0x561603e78640, _c=QMetaObject::InvokeMetaMethod, _id=4, _a=0x7fff6e344860)
    at /home/tktech/projects/kdeconnect-kde/build/interfaces/conversationsinterface.moc:138
#14 0x00007f343f73b85a in OrgKdeKdeconnectDeviceConversationsInterface::qt_metacall (this=0x561603e78640, _c=QMetaObject::InvokeMetaMethod, _id=4, _a=0x7fff6e344860)
    at /home/tktech/projects/kdeconnect-kde/build/interfaces/conversationsinterface.moc:223
#15 0x00007f343f70ae96 in DeviceConversationsDbusInterface::qt_metacall (this=0x561603e78640, _c=QMetaObject::InvokeMetaMethod, _id=10, _a=0x7fff6e344860)
    at /home/tktech/projects/kdeconnect-kde/build/interfaces/kdeconnectinterfaces_autogen/EWIEGA46WW/moc_dbusinterfaces.cpp:958
#16 0x00007f343e621300 in ?? () from /usr/lib/libQt5DBus.so.5
#17 0x00007f343d49e50f in QObject::event(QEvent*) () from /usr/lib/libQt5Core.so.5
#18 0x00007f343df46d62 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/lib/libQt5Widgets.so.5
#19 0x00007f343d4713ba in QCoreApplication::notifyInternal2(QObject*, QEvent*) () from /usr/lib/libQt5Core.so.5
#20 0x00007f343d4744b9 in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) () from /usr/lib/libQt5Core.so.5
#21 0x00007f343d4ca9b4 in ?? () from /usr/lib/libQt5Core.so.5
#22 0x00007f343b75610c in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.0
#23 0x00007f343b7a9ba9 in ?? () from /usr/lib/libglib-2.0.so.0
#24 0x00007f343b753871 in g_main_context_iteration () from /usr/lib/libglib-2.0.so.0
#25 0x00007f343d4c9fe6 in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/libQt5Core.so.5
#26 0x00007f343d46fd2c in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/libQt5Core.so.5
#27 0x00007f343d478294 in QCoreApplication::exec() () from /usr/lib/libQt5Core.so.5
#28 0x0000561602047d76 in main (argc=1, argv=0x7fff6e344f28) at /home/tktech/projects/kdeconnect-kde/smsapp/main.cpp:87

So what's the problem?

Our new and improved backtrace gives us a pretty good idea of what's going wrong, even without looking at the source code.

When trying to create a row from a message...

#8  0x00005616020551d1 in ConversationModel::createRowFromMessage

... it tries to get the first address in a list of addresses...

#7  0x0000561602056c94 in QList<ConversationAddress>::first

... which causes an assert...

#6  0x00007f343d247cf5 in qt_assert(char const*, char const*, int) ()

... which intentially kills kdeconnect-sms.

If we look at the documentation for QList::first() it's pretty clear that it causes an assert if the list is empty:

Returns a reference to the first item in the list. The list must not be empty. If the list can be empty, call isEmpty() before calling this function.

Our backtrace tells us the filename and the line where QList::first() is being called, in this case smsapp/conversationmodel.cpp on line 145:

ConversationAddress sender = message.addresses().first();
QString senderName = message.isIncoming() ? SmsHelper::getTitleForAddresses({sender}) : QString();
QString displayBody = message.body();

There may be a deeper problem here, where the list of addresses on a message should never be blank, but for now we just want to make it more resilient. The sender is only ever used to populate senderName, which looks like it can clearly handle being a blank string. So we'll add an isEmpty() guard around the call to QList::first():

QList<ConversationAddress> addresses = message.addresses();
QString senderName = QString();
if (message.isIncoming() && !addresses.isEmpty()) {
    senderName = SmsHelper::getTitleForAddresses({addresses.first()});
}
QString displayBody = message.body();

Then we rebuild our changes:

❯ make -j$(nproc) install

And after trying to open the SMS conversation that started all of this...it works!

Contributing the fix

Now that we've fixed our segfault, it would be nice if future versions of kdeconnect-sms would include these changes, so we didn't have to patch it every time we wanted to update. For that, we'll contribute our changes "upstream".

The KHeadache

A slight tangent & rant here. KDE uses a central identity service, KDE Identity. You need an account on this to make pull requests on KDE's gitlab, to comment on the forums, pretty much anything in KDE-land.

I created an account on KDE Identity in 2012. I don't remember my username. I'm not entirely sure if you could even specify a username back then, I think it was automatically created from your first and last name. I could not sign up for a new account with my regular email address, as it already existed. I cannot recover my password, as KDE Identity requires the email and the username to recover your account, and offers no way to recover your username. Clicking on the link in "Maintained by KDE Sysadmin" takes you to a bug board page that...requires a working KDE Identity account. This feels like the intro to a joke about the chicken and the egg.

Try typing "KDE identity recover username" into Google. You're going to get results like:

"Password recovery procedure is nearly impossible" - 2017
"kde identify password recovery is idiotic" - 2014
and a lot more.

All of these are rants on the inability to recover your username, or even requiring it in the first place when only an email should be required.

Apparently the "solution" is to email sysadmin@kde.org, but this address isn't actually mentioned anywhere on KDE Identity.

Things like this are a huge headache and a barrier to getting new contributors. I just about stopped right here. Alright, end of rant.

So anyways...

Our change is pretty simple, so we could probably get away with just posting it on the KDE Connect mailing list. It boils down to a short patch:

diff --git a/smsapp/conversationmodel.cpp b/smsapp/conversationmodel.cpp
index 7cd80abc..755f3796 100644
--- a/smsapp/conversationmodel.cpp
+++ b/smsapp/conversationmodel.cpp
@@ -142,8 +142,11 @@ void ConversationModel::createRowFromMessage(const ConversationMessage& message,
         return;
     }

-    ConversationAddress sender = message.addresses().first();
-    QString senderName = message.isIncoming() ? SmsHelper::getTitleForAddresses({sender}) : QString();
+    QList<ConversationAddress> addresses = message.addresses();
+    QString senderName = QString();
+    if (message.isIncoming() && !addresses.isEmpty()) {
+        senderName = SmsHelper::getTitleForAddresses({addresses.first()});
+    }
     QString displayBody = message.body();

     auto item = new QStandardItem;