From c19dfbadb27ba028a535d67d1fdb9055927ff846 Mon Sep 17 00:00:00 2001 From: jackBonadies Date: Sun, 21 Jun 2026 17:07:24 -0400 Subject: [PATCH] upload directory list can cause enumeration exceptions due to modifications on worker thread and iterating on UI thread. the list is very small so best to do copy on write semantics. --- Seeker/Settings/SettingsActivity.cs | 16 +++- Seeker/Transfers/UploadDirectoryManager.cs | 103 +++++++++++++++++---- 2 files changed, 98 insertions(+), 21 deletions(-) diff --git a/Seeker/Settings/SettingsActivity.cs b/Seeker/Settings/SettingsActivity.cs index 40cef478..fe0912c6 100644 --- a/Seeker/Settings/SettingsActivity.cs +++ b/Seeker/Settings/SettingsActivity.cs @@ -202,7 +202,7 @@ private void ClearAllFolders() // 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 @@ private void RemoveUploadDirFolder(UploadDirectoryEntry uploadDirEntry) { 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 @@ public void ParseDatabaseAndUpdateUI(Android.Net.Uri newlyAddedUriIfApplicable, { 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,7 +1101,7 @@ public void ParseDatabaseAndUpdateUI(Android.Net.Uri newlyAddedUriIfApplicable, - 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); @@ -1110,7 +1109,14 @@ public void ParseDatabaseAndUpdateUI(Android.Net.Uri newlyAddedUriIfApplicable, //throw new Exception("Directory is already added!"); } - UploadDirectoryManager.UploadDirectories.Add(newlyAddedDirectory); + if (reselectCase) + { + UploadDirectoryManager.ReplaceDirectory(UploadDirToReplaceOnReselect, newlyAddedDirectory); + } + else + { + UploadDirectoryManager.AddDirectory(newlyAddedDirectory); + } } UploadDirectoryManager.RecomputeDirectoryState(); diff --git a/Seeker/Transfers/UploadDirectoryManager.cs b/Seeker/Transfers/UploadDirectoryManager.cs index 5ed54293..0887995f 100644 --- a/Seeker/Transfers/UploadDirectoryManager.cs +++ b/Seeker/Transfers/UploadDirectoryManager.cs @@ -78,8 +78,7 @@ public static void RestoreFromSavedState(ISharedPreferences sharedPreferences) if (!string.IsNullOrEmpty(legacyUploadDataDirectory)) { var uploadDir = new UploadDirectoryEntry(new UploadDirectoryInfo(legacyUploadDataDirectory, fromTree, false, false, null)); - UploadDirectories = new List(); - UploadDirectories.Add(uploadDir); + SetDirectories(new List { uploadDir }); SaveToSharedPreferences(sharedPreferences); var editor = sharedPreferences.Edit(); @@ -88,13 +87,13 @@ public static void RestoreFromSavedState(ISharedPreferences sharedPreferences) } else { - UploadDirectories = new List(); + SetDirectories(null); } } else { var infos = SerializationHelper.DeserializeFromString>(sharedDirInfo); - UploadDirectories = infos.Select(info => new UploadDirectoryEntry(info)).ToList(); + SetDirectories(infos.Select(info => new UploadDirectoryEntry(info))); } } @@ -112,7 +111,73 @@ public static void SaveToSharedPreferences(ISharedPreferences sharedPreferences) } } - public static List 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 _uploadDirectories = new List(); + private static readonly object _uploadDirectoriesWriteLock = new object(); + + public static List UploadDirectories => _uploadDirectories; + + /// + /// Replace the whole directory list (used on restore). Snapshots the source so the caller + /// can't mutate it out from under readers afterwards. + /// + public static void SetDirectories(IEnumerable entries) + { + lock (_uploadDirectoriesWriteLock) + { + _uploadDirectories = entries == null + ? new List() + : new List(entries); + } + } + + public static void AddDirectory(UploadDirectoryEntry entry) + { + lock (_uploadDirectoriesWriteLock) + { + var copy = new List(_uploadDirectories); + copy.Add(entry); + _uploadDirectories = copy; + } + } + + public static bool RemoveDirectory(UploadDirectoryEntry entry) + { + lock (_uploadDirectoriesWriteLock) + { + var copy = new List(_uploadDirectories); + bool removed = copy.Remove(entry); + if (removed) + { + _uploadDirectories = copy; + } + return removed; + } + } + + public static void ClearDirectories() + { + lock (_uploadDirectoriesWriteLock) + { + _uploadDirectories = new List(); + } + } + + /// + /// Atomically remove and add in a + /// single swap (the reselect case), so readers never observe an intermediate state. + /// + public static void ReplaceDirectory(UploadDirectoryEntry oldEntry, UploadDirectoryEntry newEntry) + { + lock (_uploadDirectoriesWriteLock) + { + var copy = new List(_uploadDirectories); + copy.Remove(oldEntry); + copy.Add(newEntry); + _uploadDirectories = copy; + } + } public static bool IsFromTree(string presentablePath) { @@ -247,9 +312,11 @@ public static void RecomputeDirectoryState() 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 @@ public static bool IsNestedUnder(Android.Net.Uri childUri, Android.Net.Uri paren 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 @@ private static void RebuildLockedAndHiddenPrefixLists() { 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 @@ private static void RebuildLockedAndHiddenPrefixLists() 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; } }