SIGSEGV after dive computer download

[Linus Torvalds]

On Tue, Sep 17, 2013 at 10:41 AM, Thiago Macieira <thiago at macieira.org> wrote:
>
> There's only one call to QString's constructor in WeightModel::data that
> passes a non-constant string:
> [...]
> ws was initialised as:
>         weightsystem_t *ws = &current->weightsystem[index.row()];
>
> So we either have a very silly index.row() or current is a bad pointer. I
> guess we started to destroy stuff on ^Q while the UI is still updating from the
> new download.

No, I think the problem is that what *used* to be "current" simply no
longer exists, because the download replaced it with a new dive. And
"current" has changed, but "ws" is caching that *old* address within
the old "current" pointer that no longer is a valid dive.

This is why we should *not* put random pointers into the model data.
It's better to save the dive index, and then look up the data again
later. Or just about anything else, in fact.

I guess we could hide this bug by making sure we invalidate the data
very aggressively, but looking at the code, we have more serious
issues: the process_dives() call seems to be done in the context of
the downloading thread, which makes me worry about unavoidable races
with repainting during the download.

Anyway, all the other callers of process_dives() seem to do a big
reload afterwards, and the dive computerdownload  is missing code like
this:

        ui->InfoWidget->reload();
        ui->globe->reload();
        ui->ListWidget->reload(DiveTripModel::TREE);
        ui->ListWidget->setFocus();
        WSInfoModel *wsim = WSInfoModel::instance();
        wsim->updateInfo();

which is probably why that old stale pointer then stays around, and
the ^S ^Q just triggers the final repainting or something like that...

                         Linus