Re: master crash (tocwidget code)

2021-11-03 Thread Pavel Sanda
On Wed, Nov 03, 2021 at 05:18:44PM +0100, Jürgen Spitzmüller wrote:
> Am Mittwoch, dem 03.11.2021 um 16:43 +0100 schrieb Pavel Sanda:
> > Well, nothing against this particular patch, but the problem is that
> > if we accept that models_.find(type) might sucesfully return hashes
> > which are actually bogus, then all other models_.find occurences
> > might be in the same danger. So yes, this particular crash is gone,
> > but other corner cases might be lurking around.
> 
> So what do you propose instead?

I would like to play a little more to understand the logic between 
resetting and seting up the toc_. We clear it all the time and yet
we do not see the crashes, so there must be some mechanism of filling
it up immediately which fails now.

But it will take some time, time resources quite limited ATM.
Pavel
-- 
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel


Re: master crash (tocwidget code)

2021-11-03 Thread Jürgen Spitzmüller
Am Mittwoch, dem 03.11.2021 um 16:43 +0100 schrieb Pavel Sanda:
> Well, nothing against this particular patch, but the problem is that
> if we accept that models_.find(type) might sucesfully return hashes
> which are actually bogus, then all other models_.find occurences
> might be in the same danger. So yes, this particular crash is gone,
> but other corner cases might be lurking around.

So what do you propose instead?

Jürgen



signature.asc
Description: This is a digitally signed message part
-- 
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel


Re: master crash (tocwidget code)

2021-11-03 Thread Pavel Sanda
On Wed, Nov 03, 2021 at 03:59:25PM +0100, Jürgen Spitzmüller wrote:
> Am Mi., 3. Nov. 2021 um 15:17 Uhr schrieb Pavel Sanda :
> 
> > I am afraid unless we want to revamp the whole toc update machinery we just
> > shouldn't by default that expect toc_ is nonempty.
> >
> 
> This means that my patch which checks for toc_ emptiness should go in,
> right?

Well, nothing against this particular patch, but the problem is that
if we accept that models_.find(type) might sucesfully return hashes
which are actually bogus, then all other models_.find occurences
might be in the same danger. So yes, this particular crash is gone,
but other corner cases might be lurking around.

The comments in various TocModel classess/attributes are such that
I struggle to understand what they are supposed to represent and
what should I expect or not.

Pavel
-- 
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel


Re: master crash (tocwidget code)

2021-11-03 Thread Jürgen Spitzmüller
Am Mi., 3. Nov. 2021 um 15:17 Uhr schrieb Pavel Sanda :

> I am afraid unless we want to revamp the whole toc update machinery we just
> shouldn't by default that expect toc_ is nonempty.
>

This means that my patch which checks for toc_ emptiness should go in,
right?

Jürgen


>
> Pavel
> --
> lyx-devel mailing list
> lyx-devel@lists.lyx.org
> http://lists.lyx.org/mailman/listinfo/lyx-devel
>
-- 
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel


Re: master crash (tocwidget code)

2021-11-03 Thread Pavel Sanda
On Tue, Nov 02, 2021 at 10:07:20PM +0100, Pavel Sanda wrote:
> We call various reset() and clear() routines many many times,
> and its unclear to me why we repeatedlt call the whole machinery.

>From what I can see in the code the route to touching toc_ is:
Guiview::structureChanged() ->
TocModels::reset ->
TocModel::reset
TocModels::clear -> TocModel::clear

and structureChanged() get's called way more often than in the case of document
change, for example in the case of just getting focus on the lyx window or
in the case of opening empty new window (ironically the document itself is in
readonly mode).

So the likely scenario is that the moment new window is created the toc_ in the
old one gets emptied via the route above, the we load the old document in
new window which in chain of events also triggers filterContents in the old
window and you're done.


I am afraid unless we want to revamp the whole toc update machinery we just
shouldn't by default that expect toc_ is nonempty.

Pavel
-- 
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel


Re: master crash (tocwidget code)

2021-11-03 Thread Jürgen Spitzmüller
Am Mi., 3. Nov. 2021 um 13:42 Uhr schrieb Pavel Sanda :

> I can't reproduce this and do get the crash even when commenting the block
> statements except return.
>

Right. Maybe it didn't crash for me by coincidence.

Jürgen
-- 
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel


Re: master crash (tocwidget code)

2021-11-03 Thread Kornel Benko
Am Wed, 3 Nov 2021 13:41:37 +0100
schrieb Pavel Sanda :

> On Wed, Nov 03, 2021 at 08:28:59AM +0100, Jürgen Spitzmüller wrote:
> > > Maybe this code (TocWidget::updateView()) is to blame?
> > >
> > >   if (!gui_view_.documentBufferView()) {
> > >
> > >   tocTV->setModel(nullptr);
> > >
> > >   depthSL->setMaximum(0);
> > >
> > >   depthSL->setValue(0);
> > >
> > >   setEnabled(false);
> > >
> > >   return;
> > >
> > >   }
> > >
> > >  
> > Commenting this out (except for the return) at least cures the crash. Why
> > do we need to reset the model in this case?  
> 
> I can't reproduce this and do get the crash even when commenting the block
> statements except return.
> 
> Also I checked that gui_view_ is correctly different when calling for the new
> window so I would have hard time to understand what cancels toc_ content.
> 
> 
> Pavel

Not getting the crash here with latest master.

Kornel


pgpduXzpGfDoL.pgp
Description: Digitale Signatur von OpenPGP
-- 
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel


Re: master crash (tocwidget code)

2021-11-03 Thread Pavel Sanda
On Wed, Nov 03, 2021 at 08:28:59AM +0100, Jürgen Spitzmüller wrote:
> > Maybe this code (TocWidget::updateView()) is to blame?
> >
> > if (!gui_view_.documentBufferView()) {
> >
> > tocTV->setModel(nullptr);
> >
> > depthSL->setMaximum(0);
> >
> > depthSL->setValue(0);
> >
> > setEnabled(false);
> >
> > return;
> >
> > }
> >
> >
> Commenting this out (except for the return) at least cures the crash. Why
> do we need to reset the model in this case?

I can't reproduce this and do get the crash even when commenting the block
statements except return.

Also I checked that gui_view_ is correctly different when calling for the new
window so I would have hard time to understand what cancels toc_ content.


Pavel
-- 
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel


Re: master crash (tocwidget code)

2021-11-03 Thread Jürgen Spitzmüller
Am Mi., 3. Nov. 2021 um 08:14 Uhr schrieb Jürgen Spitzmüller :

> Maybe this code (TocWidget::updateView()) is to blame?
>
>   if (!gui_view_.documentBufferView()) {
>
>   tocTV->setModel(nullptr);
>
>   depthSL->setMaximum(0);
>
>   depthSL->setValue(0);
>
>   setEnabled(false);
>
>   return;
>
>   }
>
>
Commenting this out (except for the return) at least cures the crash. Why
do we need to reset the model in this case?

Jürgen
-- 
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel


Re: master crash (tocwidget code)

2021-11-03 Thread Jürgen Spitzmüller
Am Di., 2. Nov. 2021 um 22:08 Uhr schrieb Pavel Sanda :

> I believe something else is going on.
>
> When you add something like
> +++ b/src/frontends/qt/TocModel.cpp
> @@ -327,6 +330,7 @@ TocItem const TocModels::currentItem(QString const &
> type,
> }
> LASSERT(index.model() == it.value()->model(), return TocItem());
>
> + lyxerr< it.value()->toc_->empty() << "\n";
> return it.value()->tocItem(index);
>  }
>
>
> You can see steady flow of identical lines, e.g.
> tableofcontents 0x55a7b8d94af8 0x55a7b9c931f0 0
>
> associated with the first window. Then you create second window
> open new view of the same file and get steady flow of second type e.g.
>
> tableofcontents 0x55a7ba5e5438 0x55a7ba817910 0
>
> until the last line before crash
> tableofcontents 0x55a7b8d94af8 0x55a7b9c931f0 1
>
> which is again pointing to structure associated with window 1
> but now empty. So instead of missing initialization we likely
> destroy the content of first structure meanwhile.
>

Thanks, that's interesting.


> We call various reset() and clear() routines many many times,
> and its unclear to me why we repeatedlt call the whole machinery.
>

Maybe this code (TocWidget::updateView()) is to blame?

if (!gui_view_.documentBufferView()) {

tocTV->setModel(nullptr);

depthSL->setMaximum(0);

depthSL->setValue(0);

setEnabled(false);

return;

}


Jürgen
-- 
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel


Re: master crash (tocwidget code)

2021-11-02 Thread Pavel Sanda
On Tue, Nov 02, 2021 at 08:08:05AM +0100, Jürgen Spitzmüller wrote:
> Am Montag, dem 01.11.2021 um 22:51 +0100 schrieb Pavel Sanda:
> > I don't know the code in question either, but I can try to look later
> > this week.
> 
> OTOH we test for empty toc_ in several other places of the TocModel, so
> the check the patch adds might be reasonable

Yes.

> (maybe the toc is not yet re-created at that point).

I believe something else is going on.

When you add something like
+++ b/src/frontends/qt/TocModel.cpp
@@ -327,6 +330,7 @@ TocItem const TocModels::currentItem(QString const & type,
}
LASSERT(index.model() == it.value()->model(), return TocItem());
 
+ lyxerrempty() << "\n";
return it.value()->tocItem(index);
 }
 

You can see steady flow of identical lines, e.g.
tableofcontents 0x55a7b8d94af8 0x55a7b9c931f0 0

associated with the first window. Then you create second window
open new view of the same file and get steady flow of second type e.g.

tableofcontents 0x55a7ba5e5438 0x55a7ba817910 0 

until the last line before crash
tableofcontents 0x55a7b8d94af8 0x55a7b9c931f0 1

which is again pointing to structure associated with window 1
but now empty. So instead of missing initialization we likely
destroy the content of first structure meanwhile.

We call various reset() and clear() routines many many times,
and its unclear to me why we repeatedlt call the whole machinery.

My time for this issue is spent for today...
Pavel
-- 
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel


Re: master crash (tocwidget code)

2021-11-02 Thread Jürgen Spitzmüller
Am Montag, dem 01.11.2021 um 22:51 +0100 schrieb Pavel Sanda:
> I don't know the code in question either, but I can try to look later
> this week.

OTOH we test for empty toc_ in several other places of the TocModel, so
the check the patch adds might be reasonable (maybe the toc is not yet
re-created at that point).

Jürgen



signature.asc
Description: This is a digitally signed message part
-- 
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel


Re: master crash (tocwidget code)

2021-11-01 Thread Pavel Sanda
On Mon, Nov 01, 2021 at 05:01:34PM +0100, Jürgen Spitzmüller wrote:
> Am Montag, dem 01.11.2021 um 14:15 +0100 schrieb Pavel Sanda:
> > Looking at the backtrace and commit it seems that 
> > + TocItem const & item =
> > + gui_view_.tocModels().currentItem(current_type_,
> > index);
> > triggers the crash.
> > 
> > Juergen, can you reproduce?
> 
> Yes. I am not sure what goes on here exactly, but the toc seems empty
> after the window switch. I don't think the code above is to blame, it
> just reveals the problem.
> 
> The attached patch prevents the crash, but I am not sure whether it
> doesn't hide the actual problem. Do we need to update the respective
> toc somewhere? But where?

I don't know the code in question either, but I can try to look later this week.
Pavel
-- 
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel


Re: master crash (tocwidget code)

2021-11-01 Thread Jürgen Spitzmüller
Am Montag, dem 01.11.2021 um 14:15 +0100 schrieb Pavel Sanda:
> Looking at the backtrace and commit it seems that 
> + TocItem const & item =
> + gui_view_.tocModels().currentItem(current_type_,
> index);
> triggers the crash.
> 
> Juergen, can you reproduce?

Yes. I am not sure what goes on here exactly, but the toc seems empty
after the window switch. I don't think the code above is to blame, it
just reveals the problem.

The attached patch prevents the crash, but I am not sure whether it
doesn't hide the actual problem. Do we need to update the respective
toc somewhere? But where?

Jürgen

diff --git a/src/frontends/qt/TocModel.cpp b/src/frontends/qt/TocModel.cpp
index 16990b8359..c0c3dce548 100644
--- a/src/frontends/qt/TocModel.cpp
+++ b/src/frontends/qt/TocModel.cpp
@@ -327,6 +327,11 @@ TocItem const TocModels::currentItem(QString const & type,
 	}
 	LASSERT(index.model() == it.value()->model(), return TocItem());
 
+	if (it.value()->empty()) {
+		LYXERR(Debug::GUI, "TocModels::currentItem(): requested toc is empty!");
+		return TocItem();
+	}
+
 	return it.value()->tocItem(index);
 }
 
diff --git a/src/frontends/qt/TocModel.h b/src/frontends/qt/TocModel.h
index 55507a0a92..ee447616d0 100644
--- a/src/frontends/qt/TocModel.h
+++ b/src/frontends/qt/TocModel.h
@@ -47,6 +47,8 @@ public:
 	///
 	void clear();
 	///
+	bool empty() { return toc_->empty(); }
+	///
 	QAbstractItemModel * model();
 	///
 	QAbstractItemModel const * model() const;


signature.asc
Description: This is a digitally signed message part
-- 
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel


master crash (tocwidget code)

2021-11-01 Thread Pavel Sanda
Hi,

I get the crash in majority of cases when using the following recipy:
1. launch lyx & open introduction manual
2. open new window via file -> new window
3. view->hidden->introduction
4. kaboom


/usr/include/c++/10/debug/vector:434:
In function:
std::__debug::vector<_Tp, _Allocator>::const_reference 
std::__debug::vector<_Tp, 
_Allocator>::operator[](std::__debug::vector<_Tp, 
_Allocator>::size_type) const [with _Tp = lyx::TocItem; _Allocator = 
std::allocator; std::__debug::vector<_Tp, 
_Allocator>::const_reference = const lyx::TocItem&; 
std::__debug::vector<_Tp, _Allocator>::size_type = long unsigned int]

Error: attempt to subscript container with out-of-bounds index 0, but 
container only holds 0 elements.

Objects involved in the operation:
sequence "this" @ 0x0x57f3b0d0 {
  type = std::__debug::vector >;
}

Thread 1 "lyx" received signal SIGABRT, Aborted.
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
50  ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x76aa4537 in __GI_abort () at abort.c:79
#2  0x7694eeb1 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#3  0x56100051 in lyx::frontend::TocModel::tocItem(QModelIndex const&) 
const ()
#4  0x56102cbc in lyx::frontend::TocModels::currentItem(QString const&, 
QModelIndex const&) const ()
#5  0x563080c4 in lyx::frontend::TocWidget::filterContents() ()
#6  0x5630856f in lyx::frontend::TocWidget::finishUpdateView() ()
#7  0x5630947b in 
lyx::frontend::TocWidget::qt_static_metacall(QObject*, QMetaObject::Call, int, 
void**) ()


bisect leads to:

commit 4d7f4762a1af2bbe4d2051bd86e45c35361f0cfc
Author: Juergen Spitzmueller 
Date:   Sun Mar 15 10:46:35 2020 +0100

Outliner: Add filter combo for non-output items

Addresses #11442, #10786




Looking at the backtrace and commit it seems that 
+ TocItem const & item =
+ gui_view_.tocModels().currentItem(current_type_, index);
triggers the crash.

Juergen, can you reproduce?

Pavel
-- 
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel