On Fri, May 06, 2005 at 10:43:45PM +0300, Martin Vermeer wrote:
> On Fri, May 06, 2005 at 08:28:47PM +0200, Andre Poenitz wrote:
> > On Fri, May 06, 2005 at 03:59:47PM +0300, Martin Vermeer wrote:
> > > But there is no graphics or the like within a mile.
> > > It is _typing characters_ that does this. Does that ring a bell?
> >
> > Could I please have once more a recipe to produce that kind of crash?
> >
> > Andre'
>
> Open new file. Insert table. Open (right mouse) its dialog. (Or not, for
> a more challenging exercise.) Then keep the 'a' key down until a traffic
> jam builds up (one, two seconds), and let go.
>
> For the second crash, hold down the 'a' key, let go and at that very
> moment press the Down key.
The attached patch should fix the first crash properly be "faking"
reentrance of update().
It does not fix the second crash, though, but the reason seems to be
clear: <down> uses the coord cache which is invalidated at the begin
of update() and defunc up to the successful end of the outermost
update().
Note that keeping the old cache as long as the new one is constructed
is not a proper fix. Suppose we have a sequence of operations in quick
succession and swallow some updates (as e.g. done by the attached
patch). This means there are times when the buffer contains structural
changes that are not reflected in the coordcache and consequently could
lead to crashs in lfun handlers using the cache.
One solution is the proposed one of 'swallowing cache using lfuns as
long as an update is pending'. This is not just cursor up/down but also
mouse clicks and page up/down. This might work with minimal effort but
I don't think this is a proper solution (apart from fixing the crashes)
I am not sure how a proper solution would look like. We certainly need
to syncronize cache using lfuns and update() calls. Just avoiding
nested update() calls is not enough.
Andre'
Index: BufferView_pimpl.C
===================================================================
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/BufferView_pimpl.C,v
retrieving revision 1.581
diff -u -p -r1.581 BufferView_pimpl.C
--- BufferView_pimpl.C 20 Apr 2005 17:35:46 -0000 1.581
+++ BufferView_pimpl.C 7 May 2005 12:31:24 -0000
@@ -608,40 +608,74 @@ bool BufferView::Pimpl::fitCursor()
void BufferView::Pimpl::update(bool fitcursor, bool forceupdate)
{
+ // These two static variables are used to make sure that update()
+ // appears re-entrant to the outside world
+ static bool in_update = false;
+ static bool update_interrupted = false;
+
lyxerr << BOOST_CURRENT_FUNCTION
- << "[fitcursor = " << fitcursor << ','
+ << " begin, [ fitcursor = " << fitcursor << ','
<< " forceupdate = " << forceupdate
+ << " in_update = " << in_update
+ << " update_interrupted = " << update_interrupted
<< "] buffer: " << buffer_ << endl;
- // Check needed to survive LyX startup
- if (buffer_) {
- // Update macro store
- buffer_->buildMacros();
- // First drawing step
-
- CoordCache backup;
- std::swap(theCoords, backup);
- theCoords.startUpdating();
-
- ViewMetricsInfo vi = metrics();
-
- if (fitcursor && fitCursor()) {
- forceupdate = true;
- vi = metrics();
- }
- if (forceupdate) {
- // Second drawing step
- screen().redraw(*bv_, vi);
- } else {
- // Abort updating of the coord cache - just restore the
old one
+ if (in_update) {
+ // This is a nested update. We refrain from doing anything
except
+ // informing the outer level update() that something went wrong.
+ // The outer level update() is then supposed to re-do its work.
+ lyxerr << BOOST_CURRENT_FUNCTION << " suppressing update" <<
endl;
+ update_interrupted = true;
+ return;
+ }
+
+ BOOST_ASSERT(in_update == false);
+ // This is non-nested update. 'in_update' serves as a reminder
+ // that we are now updating.
+ in_update = true;
+
+ do {
+ lyxerr << BOOST_CURRENT_FUNCTION << " loop begin" << endl;
+ // So far no nested update() could run.
+ update_interrupted = false;
+
+ // Check needed to survive LyX startup
+ if (buffer_) {
+ // Update macro store
+ buffer_->buildMacros();
+
+ // First drawing step
+ CoordCache backup;
std::swap(theCoords, backup);
- }
- } else
- screen().greyOut();
-
- // And the scrollbar
- updateScrollbar();
- owner_->view_state_changed();
+ theCoords.startUpdating();
+
+ ViewMetricsInfo vi = metrics();
+
+ if (fitcursor && fitCursor()) {
+ forceupdate = true;
+ vi = metrics();
+ }
+ if (forceupdate) {
+ // Second drawing step
+ screen().redraw(*bv_, vi);
+ theCoords.doneUpdating();
+ } else {
+ // Abort updating of the coord cache - just
restore the old one
+ theCoords.doneUpdating();
+ std::swap(theCoords, backup);
+ }
+ } else
+ screen().greyOut();
+
+ // And the scrollbar
+ updateScrollbar();
+ owner_->view_state_changed();
+
+ lyxerr << BOOST_CURRENT_FUNCTION << " loop end" << endl;
+ } while (update_interrupted);
+
+ in_update = false;
+ lyxerr << BOOST_CURRENT_FUNCTION << " end " << endl;
}
