[Ide] Reduce concurrency on RecentFiles modifications.

Bug 40375 - Dozens of threads stuck trying to update the 'recent files'
file

2 files changed, 73 insertions, 47 deletions

diff --git a/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Desktop/RecentFileStorage.cs b/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Desktop/RecentFileStorage.cs
index 73a404030d..8ff55275fb 100644
--- a/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Desktop/RecentFileStorage.cs
+++ b/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Desktop/RecentFileStorage.cs
@@ -40,6 +40,7 @@ using System.Threading;
 using System.Xml;
 using System.Linq;
 using MonoDevelop.Core;
+using System.Threading.Tasks;
 
 namespace MonoDevelop.Ide.Desktop
 {
@@ -66,6 +67,13 @@ namespace MonoDevelop.Ide.Desktop
 		public RecentFileStorage (string filePath)
 		{
 			this.filePath = filePath;
+
+			// Kick off loading the filepath in the background.
+			AcquireFileExclusive (filePath).ContinueWith (t => {
+				var stream = t.Result;
+				cachedItemList = ReadStore (stream);
+				cachedItemList.Sort ();
+			});
 		}
 		
 		void EnableWatching ()
@@ -98,12 +106,12 @@ namespace MonoDevelop.Ide.Desktop
 		
 		void FileChanged (object sender, FileSystemEventArgs e)
 		{
-			OnRecentFilesChanged (null);
+			OnRecentFilesChanged (cachedItemList);
 		}
 		
 		void HandleWatcherRenamed (object sender, RenamedEventArgs e)
 		{
-			OnRecentFilesChanged (null);
+			OnRecentFilesChanged (cachedItemList);
 		}
 		
 		public bool RemoveItem (string uri)
@@ -144,15 +152,8 @@ namespace MonoDevelop.Ide.Desktop
 			if (!File.Exists (filePath)) {
 				 return new RecentItem[0];
 			}
-			
-			var c = cachedItemList;
-			if (c == null) {
-				using (var fs = AcquireFileExclusive (filePath)) {
-					c = cachedItemList = ReadStore (fs);
-				}
-			}
-			c.Sort ();
-			return c.Where (item => item.IsInGroup (group)).ToArray ();
+
+			return cachedItemList.Where (item => item.IsInGroup (group)).ToArray ();
 		}
 		
 		public void RemoveMissingFiles (params string[] groups)
@@ -208,23 +209,54 @@ namespace MonoDevelop.Ide.Desktop
 			}
 			return modified;
 		}
-		
+
+		Task recentSaveTask;
+		List<Func<List<RecentItem>, bool>> modifyList = new List<Func<List<RecentItem>, bool>> ();
 		bool ModifyStore (Func<List<RecentItem>,bool> modify)
 		{
-			List<RecentItem> list;
-			using (var fs = AcquireFileExclusive (filePath)) {
-				list = ReadStore (fs);
-				if (!modify (list)) {
-					return false;
+			lock (modifyList) {
+				modifyList.Add (modify);
+
+				// This makes recent file changed event to happen as late as possible, but it shouldn't be a problem.
+				// We keep both multiple-instance concurrency via AcquireFileExclusive lock
+				// And we batch as many modifications as possible in a 1 second window.
+				if (recentSaveTask == null) {
+					recentSaveTask = Task.Delay (1000).ContinueWith (async t => await SaveRecentFiles ());
 				}
-				fs.Position = 0;
-				fs.SetLength (0);
-				WriteStore (fs, list);
 			}
-			//TODO: can we suppress duplicate event from the FSW?
-			OnRecentFilesChanged (list);
 			return true;
 		}
+
+		async Task SaveRecentFiles ()
+		{
+			List<Func<List<RecentItem>, bool>> localModifyList;
+
+			lock (modifyList) {
+				localModifyList = new List<Func<List<RecentItem>, bool>> (modifyList);
+				modifyList.Clear ();
+				recentSaveTask = null;
+			}
+
+			using (var fs = await AcquireFileExclusive (filePath)) {
+				cachedItemList = ReadStore (fs);
+				bool modified = false;
+
+				foreach (var modify in localModifyList) {
+					if (!modify (cachedItemList)) {
+						continue;
+					}
+
+					modified = true;
+				}
+
+				if (modified) {
+					fs.Position = 0;
+					fs.SetLength (0);
+					WriteStore (fs, cachedItemList);
+					OnRecentFilesChanged (cachedItemList);
+				}
+			}
+		}
 		
 		static List<RecentItem> ReadStore (FileStream file)
 		{
@@ -267,7 +299,7 @@ namespace MonoDevelop.Ide.Desktop
 		}
 		
 		//FIXME: should we P/Invoke lockf on POSIX or is Mono's FileShare.None sufficient?
-		static FileStream AcquireFileExclusive (string filePath)
+		static async Task<FileStream> AcquireFileExclusive (string filePath)
 		{
 			const int MAX_WAIT_TIME = 1000;
 			const int RETRY_WAIT = 50;
@@ -280,7 +312,7 @@ namespace MonoDevelop.Ide.Desktop
 				} catch (Exception ex) {
 					//FIXME: will it work on Mono if we check that it's an access conflict, i.e. HResult is 0x80070020?
 					if (ex is IOException && remainingTries > 0) {
-						Thread.Sleep (RETRY_WAIT);
+						await Task.Delay (RETRY_WAIT);
 						remainingTries--;
 						continue;
 					}
diff --git a/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Desktop/RecentOpen.cs b/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Desktop/RecentOpen.cs
index 7675e92b7c..939b9dcd10 100644
--- a/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Desktop/RecentOpen.cs
+++ b/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Desktop/RecentOpen.cs
@@ -118,37 +118,31 @@ namespace MonoDevelop.Ide.Desktop
 		void Add (string grp, string fileName, string displayName)
 		{
 			var mime = DesktopService.GetMimeTypeForUri (fileName);
-			System.Threading.ThreadPool.QueueUserWorkItem (_ => {
-				try {
-					var uri = RecentFileStorage.ToUri (fileName);
-					var recentItem = new RecentItem (uri, mime, grp) { Private = displayName };
-					recentFiles.AddWithLimit (recentItem, grp, ItemLimit);
-				} catch (Exception e) {
-					LoggingService.LogError ("Failed to add item to recent files list.", e);
-				}
-			});
+			try {
+				var uri = RecentFileStorage.ToUri (fileName);
+				var recentItem = new RecentItem (uri, mime, grp) { Private = displayName };
+				recentFiles.AddWithLimit (recentItem, grp, ItemLimit);
+			} catch (Exception e) {
+				LoggingService.LogError ("Failed to add item to recent files list.", e);
+			}
 		}
 		
 		public override void NotifyFileRemoved (string fileName)
 		{
-			System.Threading.ThreadPool.QueueUserWorkItem (_ => {
-				try {
-					recentFiles.RemoveItem (RecentFileStorage.ToUri (fileName));
-				} catch (Exception e) {
-					LoggingService.LogError ("Can't remove from recent files list.", e);
-				}
-			});
+			try {
+				recentFiles.RemoveItem (RecentFileStorage.ToUri (fileName));
+			} catch (Exception e) {
+				LoggingService.LogError ("Can't remove from recent files list.", e);
+			}
 		}
 		
 		public override void NotifyFileRenamed (string oldName, string newName)
 		{
-			System.Threading.ThreadPool.QueueUserWorkItem (_ => {
-				try {
-					recentFiles.RenameItem (RecentFileStorage.ToUri (oldName), RecentFileStorage.ToUri (newName));
-				} catch (Exception e) {
-					LoggingService.LogError ("Can't rename file in recent files list.", e);
-				}
-			});
+			try {
+				recentFiles.RenameItem (RecentFileStorage.ToUri (oldName), RecentFileStorage.ToUri (newName));
+			} catch (Exception e) {
+				LoggingService.LogError ("Can't rename file in recent files list.", e);
+			}
 		}
 		
 		public void Dispose ()