On Tue, Jun 23, 2015 at 10:20:21PM +0200, Georg Baum wrote:
> Enrico Forestieri wrote:
>
> > I don't know, maybe this is a case of a race condition. Most probably,
> > with preview on, updateMacros() is not called (or is called later) and
> > the MacroData:sym_ pointer is not updated at the time MacroData::xlmname()
> > is called so that it contains bogus values. This seems to be confirmed by
> > the attached patch that explicitly sets to null the MacroData::sym_
> > pointer whenever a macro is copied. With this patch the crash is gone for
> > me.
> >
> > I think that this is to be considered as a different instance of #9418,
> > as also witnessed by the exact same backtrace.
>
> This is interesting. In theory, sym_ should only be set once, and only for
> global macros from initSymbols(). For user defined macros it should always
> be 0. According to my valgrind investigations of #9490 (which is exactly the
> same problem as reported in this thread IMHO), the crash happens because the
> MathMacro memory is already freed when MathMacro::mathmlize() is called.
> Therefore, accessing d->macro_ invokes undefined behaviour. If this is true
> then setting sym_ to 0 would only cure the symptom, but since I was unable
> up to now to pinpoint the real cause of the crash I rather do not trust my
> own findings very much anymore.
I think I understand what happens here and it seems to be a different
problem than #9490. All macros are stored using a map and when
Buffer::updateMacros() is called, it first clears the map and then
reconstructs it. The d->macro_ pointer points to a MacroData in this map
but is only updated when MathData::metrics() is called. Now, when instant
preview for math is on, MathData::metrics() is never called when
selecting a math inset, so that d->macro_ is not updated and now
d->macro_->sym_ is a bogus pointer and hence the crash when hitting Ctrl+C.
Luckily, when d->macros_ is updated by MathMacro::updateMacro(), a copy
of the pointed to data is also made. This is the data the now obsolete
d->macro_ was pointing to before Buffer::updateMacros() reorganized
things. This data contains the necessary info for trying to update
d->macro_ in the copy constructor and, if it still fails, we can simply
let d->macro_ point to that data directly.
The attached patch fixes the crash for me and I am going to commit it.
--
Enrico
diff --git a/src/mathed/MacroTable.h b/src/mathed/MacroTable.h
index 3bd04ea..030e6f7 100644
--- a/src/mathed/MacroTable.h
+++ b/src/mathed/MacroTable.h
@@ -69,6 +69,8 @@ public:
char const * MathMLtype() const;
///
void setSymbol(latexkeys const * sym) { sym_ = sym; }
+ ///
+ DocIterator const & pos() { return pos_; }
/// lock while being drawn to avoid recursions
int lock() const { return ++lockCount_; }
diff --git a/src/mathed/MathMacro.cpp b/src/mathed/MathMacro.cpp
index 24ebe77..8f2d15f 100644
--- a/src/mathed/MathMacro.cpp
+++ b/src/mathed/MathMacro.cpp
@@ -203,6 +203,14 @@ MathMacro::MathMacro(MathMacro const & that)
: InsetMathNest(that), d(new Private(*that.d))
{
d->updateChildren(this);
+ if (d->macro_ && lyxrc.preview == LyXRC::PREVIEW_ON) {
+ // We need to update d->macro_ by ourselves because in this case
+ // MathData::metrics() is not called when selecting a math inset
+ DocIterator const & pos = d->macroBackup_.pos();
+ d->macro_ = pos.buffer()->getMacro(name(), pos);
+ if (!d->macro_)
+ d->macro_ = &d->macroBackup_;
+ }
}
@@ -213,6 +221,14 @@ MathMacro & MathMacro::operator=(MathMacro const & that)
InsetMathNest::operator=(that);
*d = *that.d;
d->updateChildren(this);
+ if (d->macro_ && lyxrc.preview == LyXRC::PREVIEW_ON) {
+ // We need to update d->macro_ by ourselves because in this case
+ // MathData::metrics() is not called when selecting a math inset
+ DocIterator const & pos = d->macroBackup_.pos();
+ d->macro_ = pos.buffer()->getMacro(name(), pos);
+ if (!d->macro_)
+ d->macro_ = &d->macroBackup_;
+ }
return *this;
}
