SIGSEGV after dive computer download

Tomaz Canabrava tcanabrava at kde.org
Tue Sep 17 09:10:31 UTC 2013


On Tue, Sep 17, 2013 at 11:53 AM, Linus Torvalds <
torvalds at linux-foundation.org> wrote:

> 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.
>

Silly tomaz tougth it was not a 'random' pointer ( the Gtk version seemed
to use that too )


>
> 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...
>

Right. I *know* that this is not the right thing to do ( update everything
as something changes ) but it was easier to test while porting the Gtk
version. Since the qt version is becoming almost right, I'll take a look
into those things and will try to reduce the amount of wasted cpu cycles in
the next few days.


>
>                          Linus
> _______________________________________________
> subsurface mailing list
> subsurface at hohndel.org
> http://lists.hohndel.org/cgi-bin/mailman/listinfo/subsurface
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.hohndel.org/pipermail/subsurface/attachments/20130917/36410fc9/attachment.html>


More information about the subsurface mailing list