"Lets quickly fix this crash"

published at 09.08.2018 20:08 by Jens Weller
Save to Instapaper Pocket

A specific action causes one of my applications to crash. And so far I've just ignored it, as its not something that hinders daily use. But as I mostly get to work with code written by my self, no body else to blame or fix it. So yesterday I thought - ok - lets quickly fix this. After all I had the IDE already open...

My expectation of this being a quick fix comes from my experience, that usually something like this is caused by a small oversight of my own. Hence a short debugging session is enough to simply find the source, fix the code and avoid the crash. Not this time. The debugger first pointed at some of the innerts of sqlite, as this application is using QSql and sqlite. Then in the call stack comes first a little bit of my own code, but the values just don't make sense. Going up further, shows that an int variable is -1, which is kind of crazy, as its actually an index coming from Qt, inside of a signal handler.

More poking around lets me discover the "source":

void Sponsors_view::on_cmb_sponsors_currentIndexChanged(int index)
{
    if( current_index == -1 || index == current_index || sponsors.empty())return;
    current_index = index;
    sponsor = &sponsors[index];
    //load new values
    updateWidgets();
}

So this is a slot method, which handles the combobox currentIndexChanged signal. And its argument index is -1. My code has a different precondition, so obviously this leads to a crash. But the question here is, why and how does Qt give me such a value in this handler?! After all, it should only fire, if the selection changed, and if you select an item, the index is between 0 and number of elements -1.

The crash gets triggered, when I load a different Event(like a conference or training) into the application, which then propagates a series of handlers to reload views, one of them is new since last year: a view to handle sponsors. In this case, the view needs to reload and triggers this behavior simply by calling QComboBox::clear. And Qt then seems to trigger the slot, to let my program know that now no value is selected. Makes sense. Also a good example why having an int and not a unsigned int for this slot is good. But my code didn't expect this behavoir, and so this runs until the program crashes somewhere else. Lots of undefined behavior, but nothing that lets the debugger stop, until its really far away from the source.

Which is a bit further up the call tree, where the combobox in question gets cleared during said reload.

Luckily there is an easy way to fix this, this is the code that contains the fix now:

void Sponsors_view::loadSponsors(int eventid)
{
    current_index = -1;// I even use the same way to signal this!
    sponsors = EventDB::loadSponsor(eventid);
    const QSignalBlocker blocker(ui->cmb_sponsors);
    ui->cmb_sponsors->clear();
    ...
    updateWidgets();
}

While I also could check for the index being -1 in the handler, its of no use in my opinion. Qt has a RAII driven class that blocks all signals on an object, which is very nice if you need to reload a UI, and don't want to precheck everything within this UI for the rare event of reloading it. With QSignalBlocker simply all signals from this QComboBox instance get muted, until the object gets destroyed on the stack.

After all the fix is easy, but it took a while to understand what happens, and what exactly is the root cause of the crash. Also said application is fairly legacy, with only a few features getting added now and then.

Join the Meeting C++ patreon community!
This and other posts on Meeting C++ are enabled by my supporters on patreon!