diff --git a/FileDeduplicator.Test/DeduplicatorTests.cs b/FileDeduplicator.Test/DeduplicatorTests.cs
index a8e93b9..8888eb9 100644
--- a/FileDeduplicator.Test/DeduplicatorTests.cs
+++ b/FileDeduplicator.Test/DeduplicatorTests.cs
@@ -321,6 +321,72 @@ public void AChangedKeeperPreservesTheWholeGroup()
Assert.IsTrue(TempTree.Exists(copyTwo), "The grouped content must survive somewhere.");
}
+ ///
+ /// Deleting the other copies takes time, and the keeper can be rewritten between two
+ /// deletions. Once it no longer holds the group's content, every copy still to be deleted is
+ /// the last one left, so the rest of the group must be kept and reported.
+ ///
+ [TestMethod]
+ public void AKeeperChangedDuringTheDeleteLoopPreservesTheRemainingCopies()
+ {
+ // Arrange -- a.txt is kept; bb.txt is deleted first, then ccc.txt and dddd.txt
+ using TempTree tree = new();
+ AbsoluteFilePath keeper = tree.Write("a.txt", "shared");
+ AbsoluteFilePath first = tree.Write("bb.txt", "shared");
+ AbsoluteFilePath second = tree.Write("ccc.txt", "shared");
+ AbsoluteFilePath third = tree.Write("dddd.txt", "shared");
+ IReadOnlyList duplicates = Duplicates(FileHasher.HashFiles([keeper, first, second, third]));
+
+ // Act -- the keeper is rewritten right after the first deletion, as a sync client would
+ DeduplicationResult result = Deduplicator.DeleteDuplicates(
+ duplicates,
+ deleted =>
+ {
+ if (deleted == first)
+ {
+ _ = tree.Write("a.txt", "the keeper was overwritten mid-run");
+ }
+ });
+
+ // Assert
+ Assert.IsFalse(TempTree.Exists(first), "The copy deleted before the keeper changed is gone.");
+ Assert.IsTrue(TempTree.Exists(second), "A copy after the keeper changed may be the last one left.");
+ Assert.IsTrue(TempTree.Exists(third), "A copy after the keeper changed may be the last one left.");
+ Assert.AreEqual(1, result.DeletedCount);
+ Assert.HasCount(2, result.SkippedFiles);
+ Assert.IsTrue(result.SkippedFiles.All(s => s.Reason.Contains(keeper.WeakString, StringComparison.Ordinal)), "The reason names the keeper.");
+ }
+
+ ///
+ /// A rewrite that keeps the keeper's size and last write time slips past the cheap check
+ /// between deletions, so the keeper is re-hashed after the group's last deletion and the
+ /// loss is reported rather than passed off as a clean run.
+ ///
+ [TestMethod]
+ public void AKeeperRewrittenWithoutChangingSizeOrTimeIsReportedAfterTheGroup()
+ {
+ // Arrange
+ using TempTree tree = new();
+ AbsoluteFilePath keeper = tree.Write("a.txt", "shared");
+ AbsoluteFilePath copy = tree.Write("bb.txt", "shared");
+ IReadOnlyList duplicates = Duplicates(FileHasher.HashFiles([keeper, copy]));
+ DateTime keeperWritten = File.GetLastWriteTimeUtc(keeper.WeakString);
+
+ // Act -- same length, timestamp put back
+ DeduplicationResult result = Deduplicator.DeleteDuplicates(
+ duplicates,
+ _ =>
+ {
+ File.WriteAllText(keeper.WeakString, "SHARED");
+ File.SetLastWriteTimeUtc(keeper.WeakString, keeperWritten);
+ });
+
+ // Assert
+ Assert.AreEqual(1, result.DeletedCount);
+ Assert.ContainsSingle(result.Errors);
+ Assert.Contains(keeper.WeakString, result.Errors[0]);
+ }
+
///
/// A file that disappears before the delete pass must be reported as skipped rather than
/// counted among the deletions, so the reclaimed total stays honest.
diff --git a/FileDeduplicator/Deduplicator.cs b/FileDeduplicator/Deduplicator.cs
index 3af2feb..b01dc19 100644
--- a/FileDeduplicator/Deduplicator.cs
+++ b/FileDeduplicator/Deduplicator.cs
@@ -135,7 +135,16 @@ private static IOrderedEnumerable OrderByPreference(IEnumerabl
/// unless the group's files are empty.
internal static bool IsDeletable(DuplicateGroup group) => group.FileSize > 0;
- internal static DeduplicationResult DeleteDuplicates(IReadOnlyList duplicateGroups)
+ ///
+ /// Deletes every copy in each deletable group except the one keeps.
+ ///
+ /// The groups to deduplicate.
+ ///
+ /// Called after each copy is deleted. Production code passes nothing; tests use it to change the
+ /// filesystem between two deletions in one group.
+ ///
+ /// What was deleted, skipped and failed.
+ internal static DeduplicationResult DeleteDuplicates(IReadOnlyList duplicateGroups, Action? afterDelete = null)
{
int deletedCount = 0;
long bytesReclaimed = 0;
@@ -158,8 +167,28 @@ internal static DeduplicationResult DeleteDuplicates(IReadOnlyList f != keeper))
+ // Deleting the other copies can take seconds apiece for large files, and the keeper can
+ // be rewritten by a sync client or an editor while that runs. Once it no longer holds the
+ // group's content, every copy still to be deleted is the last one left, so the keeper is
+ // checked cheaply before each deletion and the rest of the group is kept if it moved.
+ KeeperSnapshot? keeperSnapshot = KeeperSnapshot.TryTake(keeper);
+ bool deletedFromGroup = false;
+ List others = [.. group.Files.Where(f => f != keeper)];
+
+ for (int i = 0; i < others.Count; i++)
{
+ AbsoluteFilePath file = others[i];
+
+ if (keeperSnapshot is null || !keeperSnapshot.StillMatches(keeper))
+ {
+ foreach (AbsoluteFilePath remaining in others.Skip(i))
+ {
+ Skip(remaining, $"the copy being kept ({keeper}) changed while the group was being deduplicated, so this may be the last copy of the content", skipped);
+ }
+
+ break;
+ }
+
// Re-hash immediately before deleting: a file that changed since the scan is no
// longer a duplicate, and deleting it would be irreversible loss of content that
// exists nowhere else.
@@ -183,8 +212,20 @@ internal static DeduplicationResult DeleteDuplicates(IReadOnlyList
+/// The size, last write time and identity of the copy being kept, taken once its content has been
+/// confirmed, so it can be checked cheaply between deletions without re-hashing it each time.
+///
+internal sealed class KeeperSnapshot
+{
+ private readonly long length;
+ private readonly DateTime lastWriteTimeUtc;
+ private readonly FileIdentity? identity;
+
+ private KeeperSnapshot(long length, DateTime lastWriteTimeUtc, FileIdentity? identity)
+ {
+ this.length = length;
+ this.lastWriteTimeUtc = lastWriteTimeUtc;
+ this.identity = identity;
+ }
+
+ ///
+ /// Records the keeper's current state.
+ ///
+ /// The copy being kept.
+ /// The snapshot, or if the keeper could not be read.
+ internal static KeeperSnapshot? TryTake(AbsoluteFilePath keeper) =>
+ TryRead(keeper, out long length, out DateTime lastWrite, out FileIdentity? identity)
+ ? new KeeperSnapshot(length, lastWrite, identity)
+ : null;
+
+ ///
+ /// Reports whether the keeper still looks exactly as it did when the snapshot was taken.
+ ///
+ /// The copy being kept.
+ /// if its size, last write time and identity are unchanged.
+ internal bool StillMatches(AbsoluteFilePath keeper) =>
+ TryRead(keeper, out long currentLength, out DateTime currentLastWrite, out FileIdentity? currentIdentity)
+ && currentLength == length
+ && currentLastWrite == lastWriteTimeUtc
+ && currentIdentity == identity;
+
+ private static bool TryRead(AbsoluteFilePath keeper, out long length, out DateTime lastWriteTimeUtc, out FileIdentity? identity)
+ {
+ try
+ {
+ FileInfo info = new(keeper.WeakString);
+ length = info.Length;
+ lastWriteTimeUtc = info.LastWriteTimeUtc;
+ identity = FileIdentity.TryGet(keeper.WeakString, out FileIdentity id) ? id : null;
+ return true;
+ }
+ catch (Exception ex) when (ex is IOException or UnauthorizedAccessException)
+ {
+ // A keeper that is gone or can no longer be read does not match the snapshot.
+ length = 0;
+ lastWriteTimeUtc = default;
+ identity = null;
+ return false;
+ }
+ }
+}
+
internal sealed class DuplicateGroup(string hash, List files, long fileSize)
{
internal string Hash { get; } = hash;
diff --git a/README.md b/README.md
index 55861e6..657182e 100644
--- a/README.md
+++ b/README.md
@@ -70,6 +70,8 @@ Version-control metadata is never scanned. The `.git`, `.svn`, `.hg` and `.bzr`
One file reached at two paths is not a duplicate of itself. A hardlink, a bind mount, or a share mounted twice gives one file several paths, and deleting one of them either deletes the copy being kept or frees nothing. Such paths are counted as one file, and deletion never removes a path that leads to the kept file.
+The copy being kept is checked again before each deletion. If a sync client or an editor rewrites it while the other copies are being deleted, the rest of the group is kept and reported as skipped, because each of them may now be the last copy of the content. After a group's last deletion the kept copy is re-hashed, and a change the quick check could not see is reported as an error.
+
For example, given these duplicates:
- `C:\photos\vacation\IMG_20240101_123456.jpg`
- `C:\photos\beach.jpg`