Possible fix for the mystery crash, which is bug #9049.

Investigation of bug #9236 showed that crash to be due to a Paragraph's
holding a dangling pointer to an old and deleted Layout after the
DocumentClass was reset. Since the backtraces look almost identical, it
seems likely that we have the same problem here.

Since this crash seems almost always to involve tables, I looked at the
code in switchBetweenClasses() and found that the Paragraphs that belong
to "hidden" table cells are not seen by the initial recursion using a
ParIterator: It skips right over them. This was confirmed by test code
suggested by Enrico, with results reported in Trac.

The present patch attempts to deal with this problem in the second
recursion, over Insets. When we see an InsetTabular, we call a new
routine that recurses through the cells, looking for hidden ones. If it
finds a hidden one, it then resets the Layout for the cell's Paragraphs
(there should be only one, but we do not make any assumptions) to the
PlainLayout that belongs to the new DocumentClass. This is good enough,
since such cells never have content.

There is extensive discussion of the patch here:
  https://www.mail-archive.com/lyx-devel@lists.lyx.org/msg185095.html
Additional testing by Enrico and me confirmed the existence of the
dangling pointer.
This commit is contained in:
Richard Heck 2014-08-07 15:00:35 -04:00
parent 88ce7bd5d4
commit 54c2ab2732
3 changed files with 52 additions and 26 deletions

View File

@ -737,35 +737,42 @@ void switchBetweenClasses(DocumentClassConstPtr oldone,
it->setLayout(newtc[name]);
}
// character styles
// character styles and hidden table cells
InsetIterator const i_end = inset_iterator_end(in);
for (InsetIterator it = inset_iterator_begin(in); it != i_end; ++it) {
if (it->lyxCode() != FLEX_CODE)
InsetCode const code = it->lyxCode();
if (code == FLEX_CODE) {
// FIXME: Should we verify all InsetCollapsable?
continue;
docstring const layoutName = it->layoutName();
docstring const & n = newone->insetLayout(layoutName).name();
bool const is_undefined = n.empty() ||
n == DocumentClass::plainInsetLayout().name();
if (!is_undefined)
continue;
// The flex inset is undefined in newtc
docstring const oldname = from_utf8(oldtc.name());
docstring const newname = from_utf8(newtc.name());
docstring s;
if (oldname == newname)
s = bformat(_("Flex inset %1$s is undefined after "
"reloading `%2$s' layout."), layoutName, oldname);
else
s = bformat(_("Flex inset %1$s is undefined because of "
"conversion from `%2$s' layout to `%3$s'."),
layoutName, oldname, newname);
// To warn the user that something had to be done.
errorlist.push_back(ErrorItem(
_("Undefined flex inset"),
s, it.paragraph().id(), it.pos(), it.pos() + 1));
docstring const layoutName = it->layoutName();
docstring const & n = newone->insetLayout(layoutName).name();
bool const is_undefined = n.empty() ||
n == DocumentClass::plainInsetLayout().name();
if (!is_undefined)
continue;
// The flex inset is undefined in newtc
docstring const oldname = from_utf8(oldtc.name());
docstring const newname = from_utf8(newtc.name());
docstring s;
if (oldname == newname)
s = bformat(_("Flex inset %1$s is undefined after "
"reloading `%2$s' layout."), layoutName, oldname);
else
s = bformat(_("Flex inset %1$s is undefined because of "
"conversion from `%2$s' layout to `%3$s'."),
layoutName, oldname, newname);
// To warn the user that something had to be done.
errorlist.push_back(ErrorItem(
_("Undefined flex inset"),
s, it.paragraph().id(), it.pos(), it.pos() + 1));
} else if (code == TABULAR_CODE) {
// The recursion above does not catch paragraphs in "hidden" cells,
// i.e., ones that are part of a multirow or multicolum. So we need
// to handle those separately.
// This is the cause of bug #9049.
InsetTabular * table = it->asInsetTabular();
table->setLayoutForHiddenCells(newtc);
}
}
}

View File

@ -6442,4 +6442,21 @@ string InsetTabular::params2string(InsetTabular const & inset)
}
void InsetTabular::setLayoutForHiddenCells(DocumentClass const & dc) {
for (Tabular::col_type c = 0; c < tabular.ncols(); ++c) {
for (Tabular::row_type r = 0; r < tabular.nrows(); ++r) {
if (!tabular.isPartOfMultiColumn(r,c) &&
!tabular.isPartOfMultiRow(r,c))
continue;
ParagraphList & parlist = tabular.cellInset(r,c)->paragraphs();
ParagraphList::iterator it = parlist.begin();
ParagraphList::iterator const en = parlist.end();
for (; it != en; ++it)
it->setLayout(dc.plainLayout());
}
}
}
} // namespace lyx

View File

@ -969,6 +969,8 @@ public:
/// Returns whether the cell in the specified row and column is selected.
bool isCellSelected(Cursor & cur, row_type row, col_type col) const;
///
void setLayoutForHiddenCells(DocumentClass const & dc);
//
// Public structures and variables
///