From 4c40f1ab830d869aa7be1add7879093b1d945ef2 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Sun, 8 Nov 2026 07:06:55 -0800 Subject: [PATCH] Allocate the next dashboard run ID when two dashboards start in the same millisecond (#21913) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description **What failed:** `DashboardUrlTests.DashboardRejectsWrongCrossApplicationAndBogusLoginTokens` failed 3 times on ubuntu in 7 days ([42064740887](https://github.com/microsoft/aspire/actions/runs/42064740887) on main, [41606741729](https://github.com/microsoft/aspire/actions/runs/41606741729), [42142277750](https://github.com/microsoft/aspire/actions/runs/42142277750)). `aspire-dashboard` failed to start with `InvalidOperationException: Dashboard run '' is already in use by another dashboard process` (inner `IOException` on `runs/.lock`). ## Failure stats (14d, attempt 1) `CI` workflow on microsoft/aspire, runs created 2026-10-25 06:28 → 2026-11-08 06:28 UTC, **attempt 1 only**. Denominator: attempt-1 jobs that ran (success or failure). Hits: the failed job's log matches the signature. Jobs that lost their runner, which have no log, are excluded. Main runs have no reruns, so their flake check is the next main run of the job. | | | |---|---| | Signature | `DashboardUrlTests.DashboardRejectsWrongCrossApplicationAndBogusLoginTokens` failed **and** the log has `Dashboard run '' is already in use by another dashboard process` | | Job | Hosting.Testing (all OSes) | | main pushes: hits / runs | 1 / 316 (0.32%) | | main pushes, 30d: hits / runs | 1 / 546 (0.18%). The 16 days before the 14d window add 0 hits. | | PR runs: hits / runs | 2 / 1138 (0.18%); 0 PR-caused excluded | | Combined rate | 3 / 1454 = **0.21%** | | Flake check | main: next main run of the job passed 1/1; PR: 2/2 rerun hits passed on rerun (0 never rerun) | | First / last hit | 2026-11-03 / 2026-11-06 | | Still hitting main? | Last main hit: 2026-11-05. 46 main runs since the last hit (2026-11-06), with 0 hits | | Fix evidence | Local: same-millisecond collision unit tests fail before (same exception as CI) → 20/20 after. This PR's CI: Hosting.Testing (ubuntu + windows) passed 1/1 on the head commit (plus 2/2 attempts on an earlier commit). | Failing attempt-1 jobs, most recent first (3 of 3 log-verified hits): | Date (UTC) | Branch | Job | Signature excerpt | |---|---|---|---| | [2026-11-06 01:12](https://github.com/microsoft/aspire/actions/runs/42142277750/job/127646217683) | `sebros/resource-tag-position-storage-4a8108` | Hosting.Testing (ubuntu-latest) | `Dashboard run '20261106T012000537Z' is already in use by another dashboard process` | | [2026-11-05 16:28](https://github.com/microsoft/aspire/actions/runs/42064740887/job/127379268400) | **main** | Hosting.Testing (ubuntu-latest) | `Dashboard run '20261105T163937728Z' is already in use by another dashboard process` | | [2026-11-03 08:54](https://github.com/microsoft/aspire/actions/runs/41606741729/job/125819602511) | `tommasodotnet-linux-tray-support` | Hosting.Testing (ubuntu-latest) | `Dashboard run '20261103T090228658Z' is already in use by another dashboard process` | **Verdict: MODERATE.** Low frequency (3 hits, 1 on main, all ubuntu, all passed on rerun or on the next main run), but the exact CI exception reproduces deterministically in a unit test and the root cause is a real product race. Evidence that the end-to-end test stops failing rests on that unit repro; the PR's CI sample is too small to show it. **Why:** this is a product race, not a test bug. The test starts two AppHosts with `Task.WhenAll`. In `Run` persistence mode, `DashboardRunStore` uses the start time in milliseconds as the run ID and fails fast on a lock collision. When both dashboards start in the same millisecond, the second one crashes. Any two AppHosts started together can hit this. **Fix:** `DashboardRunStore` now claims the run lock first and requires the run directory to be new. If the lock is held or the directory already exists, it moves to the next millisecond and tries again (bounded by a named `MaxRunIdAttempts` constant). This keeps the run ID format and the chronological ordinal ordering used by pruning, and it never reuses an earlier run's directory. `docs/specs/dashboard-persistence.md` is updated. Resume mode still fails fast on a lock collision, as before. Refs #21600 ## Confidence - New regression tests in `DashboardDataSourceTests`: - `RunMode_ConcurrentTimestampCollision_AllocatesNextRunId` replaces `RunMode_RejectsConcurrentTimestampCollision`, which asserted the old crash. - `RunMode_ExistingRunDirectoryWithSameTimestamp_IsNotReused` - `RunMode_RunIdAllocationExhausted_PreservesLockAcquisitionError`: if every candidate fails, the error says allocation was exhausted and keeps the underlying I/O or permission exception. Added after review feedback. It fails before that change. - **Before the fix:** the two collision tests fail. The concurrent case throws the same exception seen in CI. - **After the fix:** the collision tests pass 20/20, and `DashboardDataSourceTests` passes 64/64. - **Ad-hoc stress test (not committed):** I created 16 stores in parallel at the same fixed timestamp, 50 times. Every run ID was unique. - **Full `Aspire.Dashboard.Tests`:** 2625/2626 pass. The one failure, `OtlpGrpcServiceTests.CallService_OtlpGrpcEndPoint_ExternalFile_FileChanged_UseConfiguredKey`, is a file-watch test that also fails on unmodified main in this environment. - **Not run locally:** the end-to-end `DashboardUrlTests` (it needs DCP) and Windows/macOS. CI covers those. **How we stop this recurring:** run ID allocation now handles collisions instead of assuming they are "very unlikely", and the unit tests reproduce the same-millisecond collision deterministically with a fixed `TimeProvider`. ## Checklist - Is this feature complete? - [x] Yes. Ready to ship. - Are you including unit tests for the changes and scenario tests if relevant? - [x] Yes - Did you add public API? - [x] No - Does the change make any security assumptions or guarantees? - [x] No **🤖 Posted by AspireAgent (AI Agent)** --------- Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> (cherry picked from commit fbad38832128f625a6a28705236a55a38419acf3) --- docs/specs/dashboard-persistence.md | 5 +- .../ServiceClient/DashboardRunStore.cs | 81 ++++++++++++++++--- .../Model/DashboardDataSourceTests.cs | 50 +++++++++++- 3 files changed, 120 insertions(+), 16 deletions(-) diff --git a/docs/specs/dashboard-persistence.md b/docs/specs/dashboard-persistence.md index 8f5282a4a63..cda7a327f33 100644 --- a/docs/specs/dashboard-persistence.md +++ b/docs/specs/dashboard-persistence.md @@ -158,7 +158,7 @@ The Dashboard creates the directory with `Directory.CreateTempSubdirectory`, rem └── run.json ``` -All applications store run directories directly beneath the shared `runs` directory. Run IDs use the UTC start time in `yyyyMMddTHHmmssfffZ` format. Each run directory contains an empty file named with the application storage key. Discovery checks for this marker before opening `run.json`, avoiding metadata reads for other applications. `run.json` contains: +All applications store run directories directly beneath the shared `runs` directory. Run IDs use the UTC start time in `yyyyMMddTHHmmssfffZ` format. A Dashboard claims a run ID by taking its run lock and then creating a new run directory. If another Dashboard holds that lock or the directory already exists, such as when two AppHosts start in the same millisecond, the start time advances by one millisecond and the claim is retried. Each run directory contains an empty file named with the application storage key. Discovery checks for this marker before opening `run.json`, avoiding metadata reads for other applications. `run.json` contains: - schema version - run ID @@ -388,7 +388,8 @@ SQL values are parameterized. Schema SQL is compiled into the Dashboard assembly ## Failure behavior -- A lock collision fails startup instead of allowing two writers to share a run database. +- A run ID collision advances the run ID to the next millisecond instead of allowing two writers to share a run database. +- A resume-mode lock collision fails startup instead of allowing two writers to share an application database. - An invalid persistence mode fails Dashboard option validation and lists the accepted values. - Incomplete or unreadable historical metadata is ignored during discovery. - A historical run that disappears between discovery and selection leaves the previous run selected. diff --git a/src/Aspire.Dashboard/ServiceClient/DashboardRunStore.cs b/src/Aspire.Dashboard/ServiceClient/DashboardRunStore.cs index db20e6d08fb..87bb36dd422 100644 --- a/src/Aspire.Dashboard/ServiceClient/DashboardRunStore.cs +++ b/src/Aspire.Dashboard/ServiceClient/DashboardRunStore.cs @@ -74,6 +74,10 @@ internal sealed class DashboardRunStore : IDashboardRunStore, IDisposable private static readonly TimeSpan s_minimumLockAge = TimeSpan.FromDays(1); + // Collisions only happen when dashboards start within the same few milliseconds, so a handful of attempts is + // normally enough. The bound stops an unwritable runs directory, where every lock attempt fails, from looping forever. + internal const int MaxRunIdAttempts = 1000; + internal const string DatabaseFileName = "dashboard.db"; internal const int MaxApplicationDirectoryNameLength = 80; internal const int MaxRuns = 10; @@ -112,12 +116,7 @@ internal DashboardRunStore( var applicationName = options.Value.GetApplicationNameOrDefault(); _applicationMarkerFileName = GetApplicationDirectoryName(applicationName); var startedAt = timeProvider.GetUtcNow(); - // A millisecond timestamp collision is very unlikely. The exclusive run lock below also ensures that if two - // Dashboard instances resolve the same run ID concurrently, the second fails instead of sharing the database. - // Format invariantly. This value is a durable directory name and the ordinal sort key used by - // PruneRuns, so a non-Gregorian current culture (th-TH, ar-SA) would produce IDs that sort - // against previous runs incorrectly and let retention delete newer runs. - var runId = startedAt.ToString("yyyyMMddTHHmmssfffZ", CultureInfo.InvariantCulture); + var runId = FormatRunId(startedAt); PersistenceMode = options.Value.Data.PersistenceMode; // Persistent data can contain environment variables, telemetry, and console logs. Restrict the @@ -137,13 +136,9 @@ internal DashboardRunStore( case DashboardPersistenceMode.Run: _runsDirectory = GetRunsDirectory(options.Value.Data.Directory); DirectoryHelper.CreateWithOwnerOnlyPermissions(_runsDirectory); + var runLock = CreateNewRunDirectory(_runsDirectory, ref startedAt, out runId); CurrentWorkingDirectory = Path.Combine(_runsDirectory, runId); DatabasePath = Path.Combine(CurrentWorkingDirectory, DatabaseFileName); - // Assume that if the run directory can be created, then files in the run directory can be written as well. - Directory.CreateDirectory(CurrentWorkingDirectory); - var runLock = OpenRequiredRunLock( - CurrentWorkingDirectory, - $"Dashboard run '{runId}' is already in use by another dashboard process."); try { File.WriteAllText(Path.Combine(CurrentWorkingDirectory, _applicationMarkerFileName), string.Empty); @@ -555,6 +550,70 @@ private static bool IsPinnedRunDirectory(string runDirectory) } } + // Format invariantly. This value is a durable directory name and the ordinal sort key used by + // PruneRuns, so a non-Gregorian current culture (th-TH, ar-SA) would produce IDs that sort + // against previous runs incorrectly and let retention delete newer runs. + private static string FormatRunId(DateTimeOffset startedAt) => + startedAt.ToString("yyyyMMddTHHmmssfffZ", CultureInfo.InvariantCulture); + + /// + /// Claims a run ID that no other dashboard process has used and creates its directory. + /// + /// + /// Dashboards started in the same millisecond, such as AppHosts started together, resolve the same run ID. + /// Taking the run lock before creating the directory makes the claim atomic across processes, and requiring the + /// directory to be new prevents adopting an earlier run's data. On a collision the start time advances by one + /// millisecond, which keeps the ID format and the chronological ordinal ordering used by pruning. + /// + private static FileLock CreateNewRunDirectory(string runsDirectory, ref DateTimeOffset startedAt, out string runId) + { + Exception? lastLockException = null; + for (var attempt = 1; ; attempt++) + { + runId = FormatRunId(startedAt); + var runDirectory = Path.Combine(runsDirectory, runId); + FileLock? runLock = null; + try + { + runLock = OpenRunLock(runDirectory); + } + catch (Exception exception) when (exception is IOException or UnauthorizedAccessException) + { + // Usually another dashboard holds the lock, but this can also be a disk or permission error. + lastLockException = exception; + } + + if (runLock is not null) + { + if (!Directory.Exists(runDirectory)) + { + try + { + // Assume that if the run directory can be created, then files in the run directory can be written as well. + Directory.CreateDirectory(runDirectory); + return runLock; + } + catch + { + runLock.Dispose(); + throw; + } + } + + runLock.Dispose(); + } + + if (attempt >= MaxRunIdAttempts) + { + throw new InvalidOperationException( + $"Could not allocate a dashboard run in '{runsDirectory}' after {MaxRunIdAttempts} attempts. Each candidate run ID was locked by another dashboard process, already existed, or its lock could not be acquired. Last candidate: '{runId}'.", + lastLockException); + } + + startedAt = startedAt.AddMilliseconds(1); + } + } + private static FileLock OpenRunLock(string runDirectory) { // Keep the lock beside the run directory so pruning can hold it while recursively deleting the directory on Windows. diff --git a/tests/Aspire.Dashboard.Tests/Model/DashboardDataSourceTests.cs b/tests/Aspire.Dashboard.Tests/Model/DashboardDataSourceTests.cs index b9e298a014a..5a46bf95ae0 100644 --- a/tests/Aspire.Dashboard.Tests/Model/DashboardDataSourceTests.cs +++ b/tests/Aspire.Dashboard.Tests/Model/DashboardDataSourceTests.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Globalization; using System.Text.Json; using System.Text.Json.Nodes; using Aspire.Dashboard.Configuration; @@ -121,16 +122,59 @@ public void RunId_IsUtcTimestampWithMillisecondPrecision() } [Fact] - public void RunMode_RejectsConcurrentTimestampCollision() + public void RunMode_ConcurrentTimestampCollision_AllocatesNextRunId() { using var workspace = TemporaryWorkspace.Create(testOutputHelper); var timeProvider = new FixedTimeProvider(new DateTimeOffset(2026, 7, 20, 12, 34, 56, 789, TimeSpan.Zero)); var options = CreateOptions(workspace); using var firstRunStore = CreateRunStore(options, timeProvider); - var exception = Assert.Throws(() => CreateRunStore(options, timeProvider)); + using var secondRunStore = CreateRunStore(options, timeProvider); - Assert.Equal($"Dashboard run '{firstRunStore.RunId}' is already in use by another dashboard process.", exception.Message); + Assert.Equal("20260720T123456789Z", firstRunStore.RunId); + Assert.Equal("20260720T123456790Z", secondRunStore.RunId); + Assert.Equal(secondRunStore.RunId, Path.GetFileName(secondRunStore.CurrentWorkingDirectory)); + Assert.NotEqual(firstRunStore.DatabasePath, secondRunStore.DatabasePath); + var applicationDirectoryName = DashboardRunStore.GetApplicationDirectoryName("TestApp"); + Assert.True(File.Exists(Path.Combine(secondRunStore.CurrentWorkingDirectory, applicationDirectoryName))); + } + + [Fact] + public void RunMode_ExistingRunDirectoryWithSameTimestamp_IsNotReused() + { + using var workspace = TemporaryWorkspace.Create(testOutputHelper); + var timeProvider = new FixedTimeProvider(new DateTimeOffset(2026, 7, 20, 12, 34, 56, 789, TimeSpan.Zero)); + var existingRunDirectory = Path.Combine(DashboardRunStore.GetRunsDirectory(workspace.Path), "20260720T123456789Z"); + Directory.CreateDirectory(existingRunDirectory); + var existingFilePath = Path.Combine(existingRunDirectory, "existing.txt"); + File.WriteAllText(existingFilePath, "existing"); + + using var runStore = CreateRunStore(CreateOptions(workspace), timeProvider); + + Assert.Equal("20260720T123456790Z", runStore.RunId); + Assert.Equal("existing", File.ReadAllText(existingFilePath)); + Assert.Equal(["existing.txt"], Directory.EnumerateFileSystemEntries(existingRunDirectory).Select(Path.GetFileName)); + } + + [Fact] + public void RunMode_RunIdAllocationExhausted_PreservesLockAcquisitionError() + { + using var workspace = TemporaryWorkspace.Create(testOutputHelper); + var startedAt = new DateTimeOffset(2026, 7, 20, 12, 34, 56, 789, TimeSpan.Zero); + var timeProvider = new FixedTimeProvider(startedAt); + var runsDirectory = DashboardRunStore.GetRunsDirectory(workspace.Path); + + // A directory at each lock path makes opening the lock fail with a non-contention error. + for (var i = 0; i < DashboardRunStore.MaxRunIdAttempts; i++) + { + var runId = startedAt.AddMilliseconds(i).ToString("yyyyMMddTHHmmssfffZ", CultureInfo.InvariantCulture); + Directory.CreateDirectory(DashboardRunStore.GetRunLockPath(Path.Combine(runsDirectory, runId))); + } + + var exception = Assert.Throws(() => CreateRunStore(CreateOptions(workspace), timeProvider)); + + Assert.StartsWith($"Could not allocate a dashboard run in '{runsDirectory}' after {DashboardRunStore.MaxRunIdAttempts} attempts.", exception.Message); + Assert.IsType(exception.InnerException); } [Fact]