Add safety check to UndoStack to prevent corruption when the document is changed by UpdateStarted/UpdateFinished events within Undo() or Redo() calls.

git-svn-id: svn://svn.sharpdevelop.net/sharpdevelop/trunk@5454 1ccf3a8d-04fe-1044-b7c0-cef0b8235c61

src/Libraries/AvalonEdit/ICSharpCode.AvalonEdit/Document/DocumentChangeOperation.cs

@@ -6,13 +6,14 @@
 // </file>
 
 using System;
+using System.Diagnostics;
 
 namespace ICSharpCode.AvalonEdit.Document
 {
 	/// <summary>
 	/// Describes a change to a TextDocument.
 	/// </summary>
-	sealed class DocumentChangeOperation : IUndoableOperation
+	sealed class DocumentChangeOperation : IUndoableOperationWithContext
 	{
 		TextDocument document;
 		DocumentChangeEventArgs change;
@@ -23,6 +24,22 @@ namespace ICSharpCode.AvalonEdit.Document
 			this.change = change;
 		}
 		
+		public void Undo(UndoStack stack)
+		{
+			Debug.Assert(stack.state == UndoStack.StatePlayback);
+			stack.state = UndoStack.StatePlaybackModifyDocument;
+			this.Undo();
+			stack.state = UndoStack.StatePlayback;
+		}
+		
+		public void Redo(UndoStack stack)
+		{
+			Debug.Assert(stack.state == UndoStack.StatePlayback);
+			stack.state = UndoStack.StatePlaybackModifyDocument;
+			this.Redo();
+			stack.state = UndoStack.StatePlayback;
+		}
+		
 		public void Undo()
 		{
 			OffsetChangeMap map = change.OffsetChangeMapOrNull;

src/Libraries/AvalonEdit/ICSharpCode.AvalonEdit/Document/IUndoableOperation.cs

@@ -25,4 +25,10 @@ namespace ICSharpCode.AvalonEdit.Document
 		/// </summary>
 		void Redo();
 	}
+	
+	interface IUndoableOperationWithContext : IUndoableOperation
+	{
+		void Undo(UndoStack stack);
+		void Redo(UndoStack stack);
+	}
 }

src/Libraries/AvalonEdit/ICSharpCode.AvalonEdit/Document/UndoOperationGroup.cs

@@ -15,7 +15,7 @@ namespace ICSharpCode.AvalonEdit.Document
 	/// This class stacks the last x operations from the undostack and makes
 	/// one undo/redo operation from it.
 	/// </summary>
-	sealed class UndoOperationGroup : IUndoableOperation
+	sealed class UndoOperationGroup : IUndoableOperationWithContext
 	{
 		IUndoableOperation[] undolist;
 		
@@ -42,11 +42,25 @@ namespace ICSharpCode.AvalonEdit.Document
 			}
 		}
 		
+		public void Undo(UndoStack stack)
+		{
+			for (int i = 0; i < undolist.Length; ++i) {
+				stack.RunUndo(undolist[i]);
+			}
+		}
+		
 		public void Redo()
 		{
 			for (int i = undolist.Length - 1; i >= 0; --i) {
 				undolist[i].Redo();
 			}
 		}
+		
+		public void Redo(UndoStack stack)
+		{
+			for (int i = undolist.Length - 1; i >= 0; --i) {
+				stack.RunRedo(undolist[i]);
+			}
+		}
 	}
 }

src/Libraries/AvalonEdit/ICSharpCode.AvalonEdit/Document/UndoStack.cs

@@ -19,18 +19,32 @@ namespace ICSharpCode.AvalonEdit.Document
 	[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1711:IdentifiersShouldNotHaveIncorrectSuffix")]
 	public sealed class UndoStack : INotifyPropertyChanged
 	{
+		/// undo stack is listening for changes
+		internal const int StateListen = 0;
+		/// undo stack is reverting/repeating a set of changes
+		internal const int StatePlayback = 1;
+		// undo stack is reverting/repeating a set of changes and modifies the document to do this
+		internal const int StatePlaybackModifyDocument = 2;
+		/// state is used for checking that noone but the UndoStack performs changes
+		/// during Undo events
+		internal int state = StateListen;
+		
 		Deque<IUndoableOperation> undostack = new Deque<IUndoableOperation>();
 		Deque<IUndoableOperation> redostack = new Deque<IUndoableOperation>();
-		
 		int sizeLimit = int.MaxValue;
-		bool acceptChanges = true;
+		
+		int undoGroupDepth;
+		int actionCountInUndoGroup;
+		int optionalActionCount;
+		object lastGroupDescriptor;
+		bool allowContinue;
 		
 		/// <summary>
 		/// Gets if the undo stack currently accepts changes.
 		/// Is false while an undo action is running.
 		/// </summary>
 		public bool AcceptChanges {
-			get { return acceptChanges; }
+			get { return state == StateListen; }
 		}
 		
 		/// <summary>
@@ -77,11 +91,6 @@ namespace ICSharpCode.AvalonEdit.Document
 				redostack.PopFront();
 		}
 		
-		int undoGroupDepth;
-		int actionCountInUndoGroup;
-		int optionalActionCount;
-		object lastGroupDescriptor;
-		
 		/// <summary>
 		/// If an undo group is open, gets the group descriptor of the current top-level
 		/// undo group.
@@ -122,7 +131,7 @@ namespace ICSharpCode.AvalonEdit.Document
 		}
 		
 		/// <summary>
-		/// Starts grouping changes, continuing with the previously closed undo group.
+		/// Starts grouping changes, continuing with the previously closed undo group if possible.
 		/// Maintains a counter so that nested calls are possible.
 		/// If the call to StartContinuedUndoGroup is a nested call, it behaves exactly
 		/// as <see cref="StartUndoGroup()"/>, only top-level calls can continue existing undo groups.
@@ -132,7 +141,7 @@ namespace ICSharpCode.AvalonEdit.Document
 		public void StartContinuedUndoGroup(object groupDescriptor)
 		{
 			if (undoGroupDepth == 0) {
-				actionCountInUndoGroup = undostack.Count > 0 ? 1 : 0;
+				actionCountInUndoGroup = (allowContinue && undostack.Count > 0) ? 1 : 0;
 				optionalActionCount = 0;
 				lastGroupDescriptor = groupDescriptor;
 			}
@@ -155,21 +164,26 @@ namespace ICSharpCode.AvalonEdit.Document
 						undostack.PopBack();
 					}
 				} else if (actionCountInUndoGroup > 1) {
+					// combine all actions within the group into a single grouped action
 					undostack.PushBack(new UndoOperationGroup(undostack, actionCountInUndoGroup));
 				}
 				EnforceSizeLimit();
+				allowContinue = true;
 			}
 		}
 		
 		/// <summary>
 		/// Throws an InvalidOperationException if an undo group is current open.
 		/// </summary>
-		void VerifyNoUndoGroupOpen()
+		void ThrowIfUndoGroupOpen()
 		{
 			if (undoGroupDepth != 0) {
 				undoGroupDepth = 0;
 				throw new InvalidOperationException("No undo group should be open at this point");
 			}
+			if (state != StateListen) {
+				throw new InvalidOperationException("This method cannot be called while an undo operation is being performed");
+			}
 		}
 		
 		/// <summary>
@@ -177,14 +191,19 @@ namespace ICSharpCode.AvalonEdit.Document
 		/// </summary>
 		public void Undo()
 		{
-			VerifyNoUndoGroupOpen();
+			ThrowIfUndoGroupOpen();
 			if (undostack.Count > 0) {
-				lastGroupDescriptor = null;
-				acceptChanges = false;
+				// disallow continuing undo groups after undo operation
+				lastGroupDescriptor = null; allowContinue = false;
+				// fetch operation to undo and move it to redo stack
 				IUndoableOperation uedit = undostack.PopBack();
 				redostack.PushBack(uedit);
-				uedit.Undo();
-				acceptChanges = true;
+				state = StatePlayback;
+				try {
+					RunUndo(uedit);
+				} finally {
+					state = StateListen;
+				}
 				if (undostack.Count == 0)
 					NotifyPropertyChanged("CanUndo");
 				if (redostack.Count == 1)
@@ -192,19 +211,31 @@ namespace ICSharpCode.AvalonEdit.Document
 			}
 		}
 		
+		internal void RunUndo(IUndoableOperation op)
+		{
+			IUndoableOperationWithContext opWithCtx = op as IUndoableOperationWithContext;
+			if (opWithCtx != null)
+				opWithCtx.Undo(this);
+			else
+				op.Undo();
+		}
+		
 		/// <summary>
 		/// Call this method to redo the last undone operation
 		/// </summary>
 		public void Redo()
 		{
-			VerifyNoUndoGroupOpen();
+			ThrowIfUndoGroupOpen();
 			if (redostack.Count > 0) {
-				lastGroupDescriptor = null;
-				acceptChanges = false;
+				lastGroupDescriptor = null; allowContinue = false;
 				IUndoableOperation uedit = redostack.PopBack();
 				undostack.PushBack(uedit);
-				uedit.Redo();
-				acceptChanges = true;
+				state = StatePlayback;
+				try {
+					RunRedo(uedit);
+				} finally {
+					state = StateListen;
+				}
 				if (redostack.Count == 0)
 					NotifyPropertyChanged("CanRedo");
 				if (undostack.Count == 1)
@@ -212,6 +243,15 @@ namespace ICSharpCode.AvalonEdit.Document
 			}
 		}
 		
+		internal void RunRedo(IUndoableOperation op)
+		{
+			IUndoableOperationWithContext opWithCtx = op as IUndoableOperationWithContext;
+			if (opWithCtx != null)
+				opWithCtx.Redo(this);
+			else
+				op.Redo();
+		}
+		
 		/// <summary>
 		/// Call this method to push an UndoableOperation on the undostack.
 		/// The redostack will be cleared if you use this method.
@@ -241,7 +281,7 @@ namespace ICSharpCode.AvalonEdit.Document
 				throw new ArgumentNullException("operation");
 			}
 			
-			if (acceptChanges && sizeLimit > 0) {
+			if (state == StateListen && sizeLimit > 0) {
 				bool wasEmpty = undostack.Count == 0;
 				
 				StartUndoGroup();
@@ -272,15 +312,16 @@ namespace ICSharpCode.AvalonEdit.Document
 		/// </summary>
 		public void ClearAll()
 		{
-			VerifyNoUndoGroupOpen();
+			ThrowIfUndoGroupOpen();
+			actionCountInUndoGroup = 0;
+			optionalActionCount = 0;
 			if (undostack.Count != 0) {
 				lastGroupDescriptor = null;
+				allowContinue = false;
 				undostack.Clear();
 				NotifyPropertyChanged("CanUndo");
 			}
 			ClearRedoStack();
-			actionCountInUndoGroup = 0;
-			optionalActionCount = 0;
 		}
 		
 		internal void AttachToDocument(TextDocument document)
@@ -302,6 +343,8 @@ namespace ICSharpCode.AvalonEdit.Document
 		
 		void document_Changing(object sender, DocumentChangeEventArgs e)
 		{
+			if (state == StatePlayback)
+				throw new InvalidOperationException("Document changes during undo/redo operations are not allowed.");
 			TextDocument document = (TextDocument)sender;
 			Push(new DocumentChangeOperation(document, e));
 		}