opening qt tabular dialog when cursor is outside can crash lyx

[Juergen Spitzmueller]

Joao Luis Meloni Assirati wrote:

I managed to produce a crash by trying to invoke the tabular panel. Open
the attached document and, without moving the cursor, click with the right
mouse button on any cell of the table. This leads to a crash. Below is the
back trace with gdb.

Regards,
João.

GNU gdb 2002-04-01-cvs
Copyright 2002 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you
are
welcome to change it and/or distribute copies of it under certain
conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB. Type "show warranty" for
details.
This GDB was configured as "i386-linux"...
(gdb) run
Starting program: /work/lyx/lib/simb/lyx-qt
Qt: gdb: -nograb added to command-line options.

Use the -dograb option to enforce grabbing.

Program received signal SIGSEGV, Segmentation fault.
0x00000000 in ?? ()
(gdb) bt
#0 0x00000000 in ?? ()
#1 0x0838e5c4 in Qt2BC::setWidgetEnabled (this=0x879c3ec, obj=0x87dc950,

enabled=true) at Qt2BC.C:44

#2 0x0830a53b in GuiBC<QButton, QWidget>::refreshReadOnly
(this=0x879c3ec)

at

/usr/lib/gcc-lib/i386-linux/2.95.4/../../../../include/g++-3/stl_list.h:71
#3 0x083fa742 in ButtonControllerBase::readOnly (this=0x879c3ec,
ro=false)

at ButtonControllerBase.C:95

#4 0x084282f3 in ControlTabular::update (this=0x879c38c)

at ControlTabular.C:86

#5 0x0842821d in ControlTabular::updateInset (this=0x879c38c,
inset=0x87cab70) at ControlTabular.C:56
#6 0x0830f24d in Dialogs::updateTabular (this=0x879aa48, it=0x87cab70)

at Dialogs2.C:229

#7 0x082da2a1 in InsetTabular::resetPos (this=0x87cab70, bv=0x87a3730)

at insettabular.C:1636

#8 0x082d9d98 in InsetTabular::setPos (this=0x87cab70, bv=0x87a3730,
x=13,

y=-4) at insettabular.C:1543

#9 0x082d7c50 in InsetTabular::lfunMousePress (this=0x87cab70,

cmd=@0xbfffeed4) at insettabular.C:796

#10 0x082d82cd in InsetTabular::localDispatch (this=0x87cab70,
cmd=@0xbfffeed4)

at insettabular.C:918

#11 0x081fa6ee in LyXText::dispatch (this=0x87c9ed8, cmd=@0xbffff0c0)

at text3.C:1419

#12 0x0806d2a1 in BufferView::Pimpl::dispatch (this=0x87a3740,
ev=@0xbffff190)

at BufferView_pimpl.C:1257

#13 0x08071f79 in
boost::detail::function::void_function_obj_invoker1<boost::_bi::bind_t<bool

, boost::_mfi::mf1<bool, BufferView::Pimpl, FuncRequest const &>,

boost::_bi::list2<boost::_bi::value<BufferView::Pimpl *>, boost::arg<1> >

, void, FuncRequest>::invoke (function_obj_ptr=

{obj_ptr = 0x87a0c08, const_obj_ptr = 0x87a0c08, func_ptr =

0x87a0c08},

a0=

{view_ = 0xbffff190, action = 138846750, argument = {static npos =

4294967295, static nilRep = {len = 0, res = 0, ref = 1012, selfish =
false}, dat = 0xbffff190 ""}, x = -1073745496, y = 142261856, button_ =
1080689110})

at ../boost/boost/bind.hpp:175

#14 0x0846a22b in boost::signal1<void, FuncRequest,
boost::last_value<void>, int, less<int>, boost::function1<void,
FuncRequest, allocator<boost::function_base> > >::operator()
(this=0x87a3838, a1=0xbffff314)

at ../../../boost/boost/function/function_template.hpp:331

#15 0x08467c7c in QContentPane::mousePressEvent (this=0x87a4448,
e=0xbffff430)

at QContentPane.C:119

#16 0x402346a2 in QWidget::event () from /usr/lib/libqt.so.2
#17 0x40190ef7 in QApplication::notify () from /usr/lib/libqt.so.2
#18 0x4015c0f0 in QETWidget::translateMouseEvent () from
/usr/lib/libqt.so.2
#19 0x401599f2 in QApplication::x11ProcessEvent () from
/usr/lib/libqt.so.2
#20 0x401589cc in QApplication::processNextEvent () from
/usr/lib/libqt.so.2
#21 0x401933bf in QApplication::enter_loop () from /usr/lib/libqt.so.2
#22 0x4015ff23 in QApplication::exec () from /usr/lib/libqt.so.2
#23 0x08395a95 in lyx_gui::start (batch=@0xbffffa04, files=@0xbffff9ac)

at lyx_gui.C:169

#24 0x08140b48 in LyX::LyX (this=0xbffffa00, argc=@0xbffffa14,
argv=0xbffffa74)

at ../src/lyx_main.C:166

#25 0x081ac375 in main (argc=1, argv=0xbffffa74) at ../src/main.C:31

[Juergen Spitzmueller]

testcase

[levon]

ouch

[levon]

I can't get a useful stack trace at all.

The given trace looks very odd - we jump to 0x0 ??

Please add printfs to setWidgetEnabled to see where it's going wrong.
The cause is probably earlier

also valgrind :

==11434== Invalid read of size 4
==11434== at 0x4047EA97: QObject::inherits(char const*) const (in
/usr/lib/qt-3.0.5/lib/libqt-mt.so.3.0.5)
==11434== by 0x8204B50: Qt2BC::setWidgetEnabled(QWidget*, bool) (Qt2BC.C:40)
==11434== by 0x81D18FA: ??? (/usr/include/c++/3.2/bits/stl_list.h:98)
==11434== by 0x821679E: ButtonControllerBase::readOnly(bool)
(ButtonControllerBase.C:96)
==11434== by 0x822D733: ControlTabular::update() (ControlTabular.C:87)
==11434== by 0x822D67B: ControlTabular::updateInset(InsetTabular*)
(ControlTabular.C:56)
==11434== by 0x81C89AC: Dialogs::updateTabular(InsetTabular*) (Dialogs2.C:230)
==11434== by 0x81B10A9: InsetTabular::resetPos(BufferView*) const
(insettabular.C:1637)
==11434== by 0x81B0C3C: InsetTabular::setPos(BufferView*, int, int) const
(insettabular.C:1543)
==11434== by 0x81AF16F: InsetTabular::lfunMousePress(FuncRequest const&)
(insettabular.C:797)
==11434== by 0x81AF660: InsetTabular::localDispatch(FuncRequest const&)
(insettabular.C:919)
==11434== by 0x813B332: LyXText::dispatch(FuncRequest const&)
(/usr/include/c++/3.2/i386-redhat-linux/bits/atomicity.h:38)
==11434== by 0x806BB34: BufferView::Pimpl::dispatch(FuncRequest const&)
(/usr/include/c++/3.2/bits/basic_string.h:229)
==11434== by 0x806D04C: ??? (../boost/boost/function/function_template.hpp:129)
==11434== by 0x823AA47: ??? (QContentPane.C:112)
==11434== by 0x82397D7: QContentPane::mousePressEvent(QMouseEvent*)
(/usr/include/c++/3.2/bits/basic_string.h:229)
==11434== by 0x404B535B: QWidget::event(QEvent*) (in
/usr/lib/qt-3.0.5/lib/libqt-mt.so.3.0.5)
==11434== by 0x4042CC49: QApplication::internalNotify(QObject*, QEvent*) (in
/usr/lib/qt-3.0.5/lib/libqt-mt.so.3.0.5)
==11434== by 0x4042C363: QApplication::notify(QObject*, QEvent*) (in
/usr/lib/qt-3.0.5/lib/libqt-mt.so.3.0.5)
==11434== by 0x403D0B46: QETWidget::translateMouseEvent(_XEvent const*) (in
/usr/lib/qt-3.0.5/lib/libqt-mt.so.3.0.5)
==11434== Address 0x4B0ED094 is 0 bytes inside a block of size 132 free'd
==11434== at 0x4003BD76: builtin_delete (vg_clientfuncs.c:194)
==11434== by 0x4003BD9C: operator delete(void*) (vg_clientfuncs.c:204)
==11434== by 0x4074E833: (within /usr/lib/qt-3.0.5/lib/libqt-mt.so.3.0.5)
==11434== by 0x404B00E8: QWidget::~QWidget() (in
/usr/lib/qt-3.0.5/lib/libqt-mt.so.3.0.5)
==11434== by 0x404AFC08: QWidget::~QWidget() (in
/usr/lib/qt-3.0.5/lib/libqt-mt.so.3.0.5)
==11434== by 0x4059FADA: QWidgetStack::~QWidgetStack() (in
/usr/lib/qt-3.0.5/lib/libqt-mt.so.3.0.5)
==11434== by 0x404AFC08: QWidget::~QWidget() (in
/usr/lib/qt-3.0.5/lib/libqt-mt.so.3.0.5)
==11434== by 0x4058CA12: QTabWidget::~QTabWidget() (in
/usr/lib/qt-3.0.5/lib/libqt-mt.so.3.0.5)
==11434== by 0x404AFC08: QWidget::~QWidget() (in
/usr/lib/qt-3.0.5/lib/libqt-mt.so.3.0.5)
==11434== by 0x405DB54E: QDialog::~QDialog() (in
/usr/lib/qt-3.0.5/lib/libqt-mt.so.3.0.5)
==11434== by 0x82AAB34: QTabularDialogBase::~QTabularDialogBase()
(QTabularDialogBase.C:582)
==11434== by 0x82532C5: ??? (/usr/lib/qt3-gcc3.2/include/qstringlist.h:55)
==11434== by 0x8205132: QTabular::build_dialog()
(../../../src/frontends/controllers/ButtonController.h:38)
==11434== by 0x81D4A9D: ??? (Qt2Base.h:143)
==11434== by 0x822D66B: ControlTabular::updateInset(InsetTabular*)
(ControlTabular.C:53)
==11434== by 0x81C89AC: Dialogs::updateTabular(InsetTabular*) (Dialogs2.C:230)
==11434== by 0x81B10A9: InsetTabular::resetPos(BufferView*) const
(insettabular.C:1637)
==11434== by 0x81B0C3C: InsetTabular::setPos(BufferView*, int, int) const
(insettabular.C:1543)
==11434== by 0x81AF16F: InsetTabular::lfunMousePress(FuncRequest const&)
(insettabular.C:797)
==11434== by 0x81AF660: InsetTabular::localDispatch(FuncRequest const&)
(insettabular.C:919)

[levon]

Preliminary analysis: we are building the dialog twice,
and the widgets from the old dialog are left on the Readonly
list, causing the crash.

So I believe something is going wrong in the controller. Looking
further ...

[levon]

This is the sequence of methods in ControlTabular :

updateInset
building
showInset
show
building
update

Note that for some reason the second dialog_built_ test ("building")
is not right. Angus, can you help here ?

[leeming]

For heaven's sake what is this mess that is ControlTabular? It seems to be
nothing more than the code in ControlInset. Why not use the tested code then?

Angus cools down a little...
Ahhh

• This is pretty icky, we should really be able to use
• ControlInset. We can't because there are no params for
• tabular inset.

Then what's wrong with a dummy struct
struct TabularParams {

TabularParams(TabularParams const & other)

inset_(other.inset.clone()) {}

private:

InsetTabular inset_;

};

[levon]

because having structs that do nothing are plain confusing ?

I suppose it would be OK because we need to have params at some
point in the future, but it should be clearly commented. Fancy having
a go at this ?

Though I'd prefer analysis of the actual bug here...

[leeming]

John, I have re-written the controller class as I described and will attach it
to a new bug report when I've ironed out the bugs. In the meantime, I'm
extrememly suspicious of ControlTabular::updateInset. It circumvents the usual
control flow entirely and, anyway, it's pointless updating the status of a
dialog that isn't visible already. That said, the following code should do the
trick:

void ControlTabular::updateInset(InsetTabular * inset)
{

if (!view().isVisible())

return;

lyx::Assert(inset);
connectInset(inset);
update();

}

It requires a new method in ViewBase, but that's trivial to specialise to each
frontend and is (possibly) a good thing to have anyway.
class ViewBase {

bool isVisible() const = 0;

};

What think you?

Incidentally, I seem to remember that we moved build control into the
controllers because you were having problems with Qt. I also seem to remember
that you resolved those problems and that we could move build back into
view().show(). Is my understanding correct? The current solution is a real hack
that would be best resolved by doing that.

[levon]

My memory is fading, but I believe the issue was with update()
not build. Updating would fire signals and this would affect
the controller. I fixed it by hiding any messages to the controller
when I was inside update().

I don't remember anything off-hand about build(), sorry.

[leeming]

Ok, got it. The crash happens because you are trying to Qt2BC::setWidgetEnabled
of widgets that have not yet been created. No, I'm not going to delve deeply,
because clearly the right thing to do is as I suggested in the previous
comment. ControlTabular::update should start with:
If (!view.isVisible())

return;

I've made the patch and it works, so I'll submit it to the devel list so that as
many of you Qt guys as possible can test it.
Angus

[levon]

widgets that have not yet been created.

That's not what valgrind says. Nonetheless I'll test it.

[leeming]

I thought about it a bit more. The dialog _is_ created (that's your isbuilt pile of poo ;-) but is not visible when you do the update()). Something, somewhere probably barfs because it expects to be visible. Really, truly I don't care. The current code is designed to be wrong, so wrong is what you get... Have a good evening. I'm off for good wine and good food with some good friends around a good, big open fire. Ta-ra!

[levon]

Still doesn't sound right tbh. Anyway, the patch works, bug closed.

[leeming]

When something is broken in its design don't try and fix the superficials too
much...

Actually, the crash does make sense in some sense in that a lot of
initialisation goes on in view.show() the first time it is called and calls to
view().update() often assume that this initialisation has taken place. Calling
update() before show() just smacks of bad things waiting to happen.

Good night,
Angus