Dive download crash - Qt model issues

Dirk Hohndel dirk at hohndel.org
Sat Jun 24 10:24:03 PDT 2017


> On Jun 24, 2017, at 10:16 AM, Linus Torvalds <torvalds at linux-foundation.org> wrote:
> 
> So because I'm testing my BLE code (yes, I have working downloads on
> the Suunto EON Steel over BLE - no, it's not ready to be merged yet,
> and other dive computers won't automatically work), I've been doing
> odd things when downloading.
> 
> And I hit an issue that seems to be about the Qt models, not the
> low-level downloading code itself.
> 
> It seems reproducible to me (and happens even without the BLE code, I
> just checked), so maybe somebody else could take a look. Somebody who
> knows our Qt models (Tomaz?)
> 
> The way to cause the crash is:
> 
> - have all existing dives downloaded (but want to download again due
> to testing downloading)
> 
> - download dives, forgetting to check the "Force all dives" button
> 
> - "Arghh, nothing downloaded"
> 
> - Check the "Force all dives" button and then press "Retry download"
> 
> and when I do that, I get
> 
>  ASSERT: "last >= first" in file itemmodels/qabstractitemmodel.cpp, line 2743
>  Aborted (core dumped)

This is one of the ASSERTs in Qt that are compiled out in production code,
so I'm pretty sure those with my binaries or who are compiling from source
using standard Qt libraries won't see it.

> Now, it may be that (once more) this is because I run my self-compiled
> Qt, and apparently it ends up adding more sanity checks and asserts
> when built in developer mode. So maybe it's not so reproducible for
> others as it is for me.
> 
> The relevant gdb back-trace seems to be:
> 
> DownloadFromDCWidget::on_downloadCancelRetryButton_clicked() ->
>  DiveImportedModel::clearTable() ->
>    QAbstractItemModel::beginRemoveRows()

That should be easy to fix. In clearTable() do as the first line

if (lastIndex < firstIndex)
	return;

and you shouldn't see this crash anymore.

Tomaz, why are we setting lastIndex to -1 instead of 0 (which would seem
more reasonable to me and would avoid this assert)?

/D


More information about the subsurface mailing list