diff --git a/GitLfsCache.Tests/Storage/ObjectStoreTests.cs b/GitLfsCache.Tests/Storage/ObjectStoreTests.cs
index abc8598..8d8e94f 100644
--- a/GitLfsCache.Tests/Storage/ObjectStoreTests.cs
+++ b/GitLfsCache.Tests/Storage/ObjectStoreTests.cs
@@ -6,6 +6,7 @@ namespace ktsu.GitLfsCache.Tests.Storage;
using System.Text;
using ktsu.GitLfsCache.Configuration;
using ktsu.GitLfsCache.Storage;
+using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Abstractions;
using Microsoft.Extensions.Options;
using Microsoft.Extensions.Time.Testing;
@@ -17,6 +18,9 @@ public class ObjectStoreTests
{
private static readonly DateTimeOffset Now = new(2026, 8, 18, 12, 0, 0, TimeSpan.Zero);
+ /// The event id StoreLog.CouldNotPublishObject logs under.
+ private const int CouldNotPublishObject = 1002;
+
///
/// An absolute root for whichever platform the suite runs on. AbsoluteDirectoryPath requires a
/// fully qualified path, so a hard-coded POSIX root would be rejected on Windows.
@@ -25,7 +29,8 @@ public class ObjectStoreTests
Path.GetPathRoot(Path.GetTempPath()) ?? Path.DirectorySeparatorChar.ToString(),
"gitlfscache-tests");
- private static (ObjectStore Store, MockFileSystem FileSystem, FakeTimeProvider Time) Create()
+ private static (ObjectStore Store, MockFileSystem FileSystem, FakeTimeProvider Time) Create(
+ ILogger? logger = null)
{
MockFileSystem fileSystem = new();
fileSystem.Directory.CreateDirectory(Root);
@@ -37,7 +42,11 @@ private static (ObjectStore Store, MockFileSystem FileSystem, FakeTimeProvider T
FakeTimeProvider time = new(Now);
- ObjectStore store = new(fileSystem, Options.Create(options), time, NullLogger.Instance);
+ ObjectStore store = new(
+ fileSystem,
+ Options.Create(options),
+ time,
+ logger ?? NullLogger.Instance);
return (store, fileSystem, time);
}
@@ -127,6 +136,44 @@ public async Task PublishAsync_ObjectAlreadyPresent_SucceedsAndRemovesStaging()
Assert.IsFalse(fileSystem.File.Exists(stagingPath));
}
+ [TestMethod]
+ public async Task PublishAsync_ObjectArrivesBetweenTheCheckAndTheMove_SucceedsWithoutReportingAFailure()
+ {
+ RecordingLogger logger = new();
+ (ObjectStore store, MockFileSystem fileSystem, _) = Create(logger);
+ (byte[] content, string oid) = Content("published twice at once");
+ string destination = fileSystem.Path.Combine(Root, "github", "objects", oid[..2], oid[2..4], oid);
+
+ StagingHandle handle = store.OpenStaging("github");
+ string stagingPath = handle.Path;
+ await handle.Stream.WriteAsync(content, CancellationToken.None);
+
+ // Two uploads of one blob are not coordinated, so both can pass the exists check and then both
+ // rename. Publishing the winner's copy from inside this one's rename puts the object in place
+ // in exactly that window, which is what the loser of the real race finds.
+ bool raced = false;
+ fileSystem.Intercept.Event(
+ _ =>
+ {
+ raced = true;
+ fileSystem.File.WriteAllBytes(destination, content);
+ },
+ change => !raced
+ && change.ChangeType == WatcherChangeTypes.Renamed
+ && string.Equals(change.Path, fileSystem.Path.GetFullPath(destination), StringComparison.Ordinal));
+
+ bool published = await store.PublishAsync(handle, "github", oid, CancellationToken.None);
+
+ Assert.IsTrue(raced, "The other publisher never ran, so the race was not reproduced.");
+ Assert.IsTrue(published, "Losing the race to a byte-identical object is not a publish failure.");
+ Assert.IsTrue(store.Exists("github", oid));
+ Assert.IsFalse(fileSystem.File.Exists(stagingPath), "Staging must not survive the race.");
+ CollectionAssert.DoesNotContain(
+ logger.Events,
+ CouldNotPublishObject,
+ "A duplicate publish must not be reported as a publish failure.");
+ }
+
[TestMethod]
public async Task StagingHandle_DisposedWithoutPublishing_DeletesTheFile()
{
@@ -329,4 +376,24 @@ public void EnumerateStaging_NoStoreYet_ReturnsNothing()
Assert.HasCount(0, store.Enumerate().ToList());
Assert.HasCount(0, store.EnumerateStaging().ToList());
}
+
+ ///
+ /// Records the event ids the store logs, so a test can assert on what it did not report as well
+ /// as on what it did.
+ ///
+ private sealed class RecordingLogger : ILogger
+ {
+ public List Events { get; } = [];
+
+ public IDisposable? BeginScope(TState state) where TState : notnull => null;
+
+ public bool IsEnabled(LogLevel logLevel) => true;
+
+ public void Log(
+ LogLevel logLevel,
+ EventId eventId,
+ TState state,
+ Exception? exception,
+ Func formatter) => Events.Add(eventId.Id);
+ }
}
diff --git a/GitLfsCache/Storage/ObjectStore.cs b/GitLfsCache/Storage/ObjectStore.cs
index b4ab6da..a892d41 100644
--- a/GitLfsCache/Storage/ObjectStore.cs
+++ b/GitLfsCache/Storage/ObjectStore.cs
@@ -157,6 +157,19 @@ public async Task PublishAsync(
}
catch (Exception failure) when (failure is IOException or UnauthorizedAccessException)
{
+ // The check above and the rename are not one atomic step, so another request publishing
+ // the same object can land between them and the rename then fails on an occupied
+ // destination. Uploads never go through the fetch coalescer, so two clients pushing one
+ // blob reach here with nothing coordinating them. Content addressing makes the file that
+ // arrived byte-identical to this one, which is the same outcome the check reports: the
+ // winner stands. Reporting it as a failure would have the caller record a verification
+ // failure, and a benign race is not something to alert on.
+ if (fileSystem.File.Exists(destination))
+ {
+ await handle.DisposeAsync().ConfigureAwait(false);
+ return true;
+ }
+
StoreLog.CouldNotPublishObject(logger, failure, oid, upstream);
await handle.DisposeAsync().ConfigureAwait(false);
return false;