Le 27/12/2016 à 23:11, Scott Kostyshak a écrit :
On Tue, May 31, 2016 at 01:15:38AM +0200, Guillaume Munch wrote:
commit 1cc14a31ca8320d881b674f93c34a09cf1666cca
Author: Guillaume Munch <g...@lyx.org>
Date: Mon May 30 21:42:08 2016 +0100
TocWidget: fix an erroneous collapse and optimise updates based on profiling
TocModels::reset() in GuiView::structureChanged() collapses the TocWidget,
and
therefore requires an update right after, which was missing.
In fact, profiling TocWidget::updateView() shows that delaying the update is
good only for fast keypresses (essentially movement). It costs 5% of a
char-forward operation in a document with approx. 100 table of contents
items. The update optimisation has been rewritten to take this data into
account.
A git bisect suggests this causes a SIGSEGV. To reproduce:
1. open the attached file
2. open the outline pane
3. put the cursor just to the left of "Hello" (but inside the caption inset)
4. "Save As" to a new filename.
Can you reproduce?
Backtrace is attached.
Hi,
The root of the issue is in Buffer::reload() called during "Save As".
After loadLyXFile() there, all the insets have been deleted, and
therefore the Inset pointer
GuiView::documentBufferView()->cursor().inset() is dangling. Immediately
after loadLyXFile(), reload() calls updateBuffer() from your backtrace
which then crashes.
While debugging I got other segfaults caused by the same dangling Inset
pointer in Cursor, notably: 1) a trace identical to the second one from
<http://www.lyx.org/trac/ticket/10520>, and 2) a repeated segfault in
the critical path (a call to inMathed()) which closes LyX instantly
after the first segfault is caught (luckily after the emergency save is
complete).
I am trying to find a proper fix for this issue, but the strategy for
managing these Inset pointers in CursorSlice escapes me. Does anyone
know which assumptions are made on when these pointers must be valid?
If there are such assumptions, how are they supposed to be enforced?
The attached patch fixes this crash, but I find very unsatisfactory to
have to update the cursors by hand like this.
Guillaume
diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index 3652e7a..e45efd1 100644
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -1044,6 +1044,9 @@ bool Buffer::readDocument(Lexer & lex)
bool const res = text().read(lex, errorList, d->inset);
d->old_position.clear();
+ // Insets have been deleted; update the inset_ cache of Cursors.
+ updateCursors();
+
// inform parent buffer about local macros
if (parent()) {
Buffer const * pbuf = parent();
@@ -3816,6 +3819,13 @@ ErrorList & Buffer::errorList(string const & type) const
}
+void Buffer::updateCursors() const
+{
+ if (d->gui_)
+ d->gui_->updateCursors();
+}
+
+
void Buffer::updateTocItem(std::string const & type,
DocIterator const & dit) const
{
diff --git a/src/Buffer.h b/src/Buffer.h
index 0cb7026..ba68a45 100644
--- a/src/Buffer.h
+++ b/src/Buffer.h
@@ -682,6 +682,8 @@ public:
bool lastPreviewError() const;
private:
+ /// This function is called when inset pointers have changed.
+ void updateCursors() const;
///
ExportStatus doExport(std::string const & target, bool put_in_tempdir,
std::string & result_file) const;
diff --git a/src/BufferView.cpp b/src/BufferView.cpp
index d017db0..81b6d46 100644
--- a/src/BufferView.cpp
+++ b/src/BufferView.cpp
@@ -2447,6 +2447,12 @@ int BufferView::workHeight() const
}
+void BufferView::updateCursorInsets()
+{
+ cursor().updateInsets(&buffer().inset());
+}
+
+
void BufferView::setCursor(DocIterator const & dit)
{
d->cursor_.reset();
diff --git a/src/BufferView.h b/src/BufferView.h
index ee2de82..f17b57e 100644
--- a/src/BufferView.h
+++ b/src/BufferView.h
@@ -247,6 +247,8 @@ public:
Cursor & cursor();
/// access to full cursor.
Cursor const & cursor() const;
+ /// update the cursor inset cache
+ void updateCursorInsets();
/// sets cursor.
/// This will also open all relevant collapsable insets.
void setCursor(DocIterator const &);
diff --git a/src/frontends/Delegates.h b/src/frontends/Delegates.h
index d13af7e..0f969e6 100644
--- a/src/frontends/Delegates.h
+++ b/src/frontends/Delegates.h
@@ -72,6 +72,8 @@ public:
virtual void setBusy(bool) = 0;
/// Reset autosave timers for all users.
virtual void resetAutosaveTimers() = 0;
+ /// This function is called when inset pointers have changed.
+ virtual void updateCursors() = 0;
};
} // namespace frontend
diff --git a/src/frontends/qt4/GuiView.cpp b/src/frontends/qt4/GuiView.cpp
index 90b43eb..8bb699a 100644
--- a/src/frontends/qt4/GuiView.cpp
+++ b/src/frontends/qt4/GuiView.cpp
@@ -1661,6 +1661,15 @@ void GuiView::structureChanged()
}
+void GuiView::updateCursors()
+{
+ if (documentBufferView())
+ documentBufferView()->updateCursorInsets();
+ if (currentBufferView() && documentBufferView() != currentBufferView())
+ currentBufferView()->updateCursorInsets();
+}
+
+
void GuiView::updateDialog(string const & name, string const & data)
{
if (!isDialogVisible(name))
diff --git a/src/frontends/qt4/GuiView.h b/src/frontends/qt4/GuiView.h
index a864f4b..58c0202 100644
--- a/src/frontends/qt4/GuiView.h
+++ b/src/frontends/qt4/GuiView.h
@@ -107,10 +107,6 @@ public:
void newDocument(std::string const & filename,
bool fromTemplate);
- /// display a message in the view
- /// could be called from any thread
- void message(docstring const &);
-
bool getStatus(FuncRequest const & cmd, FuncStatus & flag);
/// dispatch command.
/// \return true if the \c FuncRequest has been dispatched.
@@ -167,6 +163,10 @@ public:
void errors(std::string const &, bool from_master = false);
void structureChanged();
void updateTocItem(std::string const &, DocIterator const &);
+ void updateCursors();
+ /// display a message in the view
+ /// could be called from any thread
+ void message(docstring const &);
//@}
///
