Fix upload dirs enumeration exceptions

Seeker/Settings/SettingsActivity.cs

@@ -202,7 +202,7 @@
             // If a parse is in flight, cancel it FIRST so it can't finish and clobber the cleared state by
             // committing / persisting its (now-stale) cache. No-op when nothing is parsing.
             SharedFileService.CancelOngoingParse();
-            UploadDirectoryManager.UploadDirectories.Clear();
+            UploadDirectoryManager.ClearDirectories();
             UploadDirectoryManager.SaveToSharedPreferences(SeekerState.SharedPreferences);
             SharedFileService.ClearFileCache();
         }
@@ -644,7 +644,7 @@
             {
                 System.Threading.ThreadPool.QueueUserWorkItem((object o) =>
                 {
-                    UploadDirectoryManager.UploadDirectories.Remove(uploadDirEntry);
+                    UploadDirectoryManager.RemoveDirectory(uploadDirEntry);
 
                     // remove purely in memory, no disk walk; on failure fall back to a full rescan like before
                     bool removed = SharedFileService.TryRemoveSharedFolderInMemory(uploadDirEntry, out var removalErr);
@@ -1092,7 +1092,6 @@
                 {
                     newlyAddedDirectory = new UploadDirectoryEntry(new UploadDirectoryInfo(newlyAddedUriIfApplicable.ToString(), !fromLegacyPicker, UploadDirToReplaceOnReselect.Info.IsLocked, UploadDirToReplaceOnReselect.Info.IsHidden, UploadDirToReplaceOnReselect.Info.DisplayNameOverride));
                     newlyAddedDirectory.UploadDirectory = fromLegacyPicker ? DocumentFile.FromFile(new Java.IO.File(newlyAddedUriIfApplicable.Path)) : DocumentFile.FromTreeUri(this, newlyAddedUriIfApplicable);
-                    UploadDirectoryManager.UploadDirectories.Remove(UploadDirToReplaceOnReselect);
                 }
                 else
                 {
@@ -1102,15 +1101,22 @@
 
 
 
-                if (UploadDirectoryManager.UploadDirectories.Where(up => up.Info.UploadDataDirectoryUri == newlyAddedUriIfApplicable.ToString()).Count() != 0)
+                if (UploadDirectoryManager.UploadDirectories.Where(up => up.Info.UploadDataDirectoryUri == newlyAddedUriIfApplicable.ToString() && (!reselectCase || up != UploadDirToReplaceOnReselect)).Count() != 0)
                 {
                     //error!!
                     SeekerApplication.Toaster.ShowToast(SeekerApplication.GetString(Resource.String.ErrorAlreadyAdded), ToastLength.Long);
                     return;
                     //throw new Exception("Directory is already added!");
                 }
 
-                UploadDirectoryManager.UploadDirectories.Add(newlyAddedDirectory);
+                if (reselectCase)
+                {
+                    UploadDirectoryManager.ReplaceDirectory(UploadDirToReplaceOnReselect, newlyAddedDirectory);
+                }
+                else
+                {
+                    UploadDirectoryManager.AddDirectory(newlyAddedDirectory);
+                }
             }
 
             UploadDirectoryManager.RecomputeDirectoryState();

Seeker/Transfers/UploadDirectoryManager.cs

@@ -78,8 +78,7 @@
                 if (!string.IsNullOrEmpty(legacyUploadDataDirectory))
                 {
                     var uploadDir = new UploadDirectoryEntry(new UploadDirectoryInfo(legacyUploadDataDirectory, fromTree, false, false, null));
-                    UploadDirectories = new List<UploadDirectoryEntry>();
-                    UploadDirectories.Add(uploadDir);
+                    SetDirectories(new List<UploadDirectoryEntry> { uploadDir });
 
                     SaveToSharedPreferences(sharedPreferences);
                     var editor = sharedPreferences.Edit();
@@ -88,13 +87,13 @@
                 }
                 else
                 {
-                    UploadDirectories = new List<UploadDirectoryEntry>();
+                    SetDirectories(null);
                 }
             }
             else
             {
                 var infos = SerializationHelper.DeserializeFromString<List<UploadDirectoryInfo>>(sharedDirInfo);
-                UploadDirectories = infos.Select(info => new UploadDirectoryEntry(info)).ToList();
+                SetDirectories(infos.Select(info => new UploadDirectoryEntry(info)));
             }
         }
 
@@ -112,7 +111,73 @@
             }
         }
 
-        public static List<UploadDirectoryEntry> UploadDirectories { get; private set; }
+        // Copy-on-write: UploadDirectories is shared across the UI thread and multiple ThreadPool
+        // background threads (folder add/remove, parse/rescan).
+        private static volatile List<UploadDirectoryEntry> _uploadDirectories = new List<UploadDirectoryEntry>();
+        private static readonly object _uploadDirectoriesWriteLock = new object();
+
+        public static List<UploadDirectoryEntry> UploadDirectories => _uploadDirectories;
+
+        /// <summary>
+        /// Replace the whole directory list (used on restore). Snapshots the source so the caller
+        /// can't mutate it out from under readers afterwards.
+        /// </summary>
+        public static void SetDirectories(IEnumerable<UploadDirectoryEntry> entries)
+        {
+            lock (_uploadDirectoriesWriteLock)
+            {
+                _uploadDirectories = entries == null
+                    ? new List<UploadDirectoryEntry>()
+                    : new List<UploadDirectoryEntry>(entries);
+            }
+        }
+
+        public static void AddDirectory(UploadDirectoryEntry entry)
+        {
+            lock (_uploadDirectoriesWriteLock)
+            {
+                var copy = new List<UploadDirectoryEntry>(_uploadDirectories);
+                copy.Add(entry);
+                _uploadDirectories = copy;
+            }
+        }
+
+        public static bool RemoveDirectory(UploadDirectoryEntry entry)
+        {
+            lock (_uploadDirectoriesWriteLock)
+            {
+                var copy = new List<UploadDirectoryEntry>(_uploadDirectories);
+                bool removed = copy.Remove(entry);
+                if (removed)
+                {
+                    _uploadDirectories = copy;
+                }
+                return removed;
+            }
+        }
+
+        public static void ClearDirectories()
+        {
+            lock (_uploadDirectoriesWriteLock)
+            {
+                _uploadDirectories = new List<UploadDirectoryEntry>();
+            }
+        }
+
+        /// <summary>
+        /// Atomically remove <paramref name="oldEntry"/> and add <paramref name="newEntry"/> in a
+        /// single swap (the reselect case), so readers never observe an intermediate state.
+        /// </summary>
+        public static void ReplaceDirectory(UploadDirectoryEntry oldEntry, UploadDirectoryEntry newEntry)
+        {
+            lock (_uploadDirectoriesWriteLock)
+            {
+                var copy = new List<UploadDirectoryEntry>(_uploadDirectories);
+                copy.Remove(oldEntry);
+                copy.Add(newEntry);
+                _uploadDirectories = copy;
+            }
+        }
 
         public static bool IsFromTree(string presentablePath)
         {
@@ -247,9 +312,11 @@
 
         private static void ResolveDocumentFilesAndErrorStates()
         {
-            for (int i = 0; i < UploadDirectories.Count; i++)
+            // Snapshot once so a concurrent copy-on-write swap can't tear Count vs. [i].
+            var dirs = _uploadDirectories;
+            for (int i = 0; i < dirs.Count; i++)
             {
-                UploadDirectoryEntry entry = UploadDirectories[i];
+                UploadDirectoryEntry entry = dirs[i];
 
                 Android.Net.Uri uploadDirUri = Android.Net.Uri.Parse(entry.Info.UploadDataDirectoryUri);
                 try
@@ -301,17 +368,19 @@
 
         private static void RecomputeSubdirFlags()
         {
-            for (int i = 0; i < UploadDirectories.Count; i++)
+            // Snapshot once so a concurrent copy-on-write swap can't tear Count vs. [i].
+            var dirs = _uploadDirectories;
+            for (int i = 0; i < dirs.Count; i++)
             {
-                UploadDirectoryEntry entry = UploadDirectories[i];
+                UploadDirectoryEntry entry = dirs[i];
                 var ourUri = Android.Net.Uri.Parse(entry.Info.UploadDataDirectoryUri);
 
                 entry.IsSubdir = false;
-                for (int j = 0; j < UploadDirectories.Count; j++)
+                for (int j = 0; j < dirs.Count; j++)
                 {
                     if (i != j)
                     {
-                        if (IsNestedUnder(ourUri, Android.Net.Uri.Parse(UploadDirectories[j].Info.UploadDataDirectoryUri)))
+                        if (IsNestedUnder(ourUri, Android.Net.Uri.Parse(dirs[j].Info.UploadDataDirectoryUri)))
                         {
                             entry.IsSubdir = true;
                         }
@@ -324,9 +393,11 @@
         {
             PresentableNameLockedDirectories.Clear();
             PresentableNameHiddenDirectories.Clear();
-            for (int i = 0; i < UploadDirectories.Count; i++)
+            // Snapshot once so a concurrent copy-on-write swap can't tear Count vs. [i].
+            var dirs = _uploadDirectories;
+            for (int i = 0; i < dirs.Count; i++)
             {
-                UploadDirectoryEntry entry = UploadDirectories[i];
+                UploadDirectoryEntry entry = dirs[i];
                 if (!entry.Info.IsLocked && !entry.Info.IsHidden)
                 {
                     continue;
@@ -350,13 +421,13 @@
 
                     UploadDirectoryEntry ourTopLevelParent = null;
 
-                    for (int j = 0; j < UploadDirectories.Count; j++)
+                    for (int j = 0; j < dirs.Count; j++)
                     {
                         if (i != j)
                         {
-                            if (!UploadDirectories[j].IsSubdir && IsNestedUnder(ourUri, Android.Net.Uri.Parse(UploadDirectories[j].Info.UploadDataDirectoryUri)))
+                            if (!dirs[j].IsSubdir && IsNestedUnder(ourUri, Android.Net.Uri.Parse(dirs[j].Info.UploadDataDirectoryUri)))
                             {
-                                ourTopLevelParent = UploadDirectories[j];
+                                ourTopLevelParent = dirs[j];
                                 break;
                             }
                         }