From 54c2ab2732f8094c0c119618d5b0e9a625ffb262 Mon Sep 17 00:00:00 2001 From: Richard Heck Date: Thu, 7 Aug 2014 15:00:35 -0400 Subject: [PATCH] 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. --- src/CutAndPaste.cpp | 59 +++++++++++++++++++++---------------- src/insets/InsetTabular.cpp | 17 +++++++++++ src/insets/InsetTabular.h | 2 ++ 3 files changed, 52 insertions(+), 26 deletions(-) diff --git a/src/CutAndPaste.cpp b/src/CutAndPaste.cpp index c3367215e3..ad3ad7e238 100644 --- a/src/CutAndPaste.cpp +++ b/src/CutAndPaste.cpp @@ -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); + } } } diff --git a/src/insets/InsetTabular.cpp b/src/insets/InsetTabular.cpp index ac8e723d87..2a9c7e64b7 100644 --- a/src/insets/InsetTabular.cpp +++ b/src/insets/InsetTabular.cpp @@ -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 diff --git a/src/insets/InsetTabular.h b/src/insets/InsetTabular.h index 2763eef0b8..0aac6420ec 100644 --- a/src/insets/InsetTabular.h +++ b/src/insets/InsetTabular.h @@ -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 /// -- 2.39.2