From dc00e5388e83a1a92cccfa6fa6c6a42863088645 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Tue, 2 Jun 2026 16:07:02 +0900 Subject: [PATCH] Fix atomic state writes for #2828 --- changelog.d/unreleased/2828.fixed.md | 21 +++++ src/CodeIndex/Cli/AtomicFileWriter.cs | 77 +++++++++++++++++++ src/CodeIndex/Cli/DataDirectorySecurity.cs | 7 +- .../Cli/IndexCommandRunner.FullScan.cs | 2 +- src/CodeIndex/Cli/SuggestionStore.cs | 22 +----- src/CodeIndex/Cli/UpdateChecker.cs | 2 +- .../DataDirectorySecurityTests.cs | 23 ++++++ tests/CodeIndex.Tests/SuggestionStoreTests.cs | 5 +- 8 files changed, 128 insertions(+), 31 deletions(-) create mode 100644 changelog.d/unreleased/2828.fixed.md create mode 100644 src/CodeIndex/Cli/AtomicFileWriter.cs diff --git a/changelog.d/unreleased/2828.fixed.md b/changelog.d/unreleased/2828.fixed.md new file mode 100644 index 0000000000..13ce9b0f79 --- /dev/null +++ b/changelog.d/unreleased/2828.fixed.md @@ -0,0 +1,21 @@ +--- +category: fixed +issues: + - 2828 +affected: + - src/CodeIndex/Cli/AtomicFileWriter.cs + - src/CodeIndex/Cli/DataDirectorySecurity.cs + - src/CodeIndex/Cli/SuggestionStore.cs + - src/CodeIndex/Cli/UpdateChecker.cs + - src/CodeIndex/Cli/IndexCommandRunner.FullScan.cs + - tests/CodeIndex.Tests/DataDirectorySecurityTests.cs + - tests/CodeIndex.Tests/SuggestionStoreTests.cs +--- + +## English + +- **Small persistent state writes now use flushed atomic replacement (#2828)** — active workspace, lock metadata, suggestion store, update-cache, and scan-checkpoint writes now go through a temp-file, flush-to-disk, and rename path with best-effort temp cleanup on failure. + +## 日本語 + +- **小さな永続状態ファイルの書き込みをflush付きatomic置換にしました (#2828)** — active workspace、lock metadata、suggestion store、update-cache、scan-checkpoint の書き込みは、一時ファイル、ディスクflush、renameを経由し、失敗時は一時ファイルをベストエフォートで掃除するようになりました。 diff --git a/src/CodeIndex/Cli/AtomicFileWriter.cs b/src/CodeIndex/Cli/AtomicFileWriter.cs new file mode 100644 index 0000000000..85cebd4d17 --- /dev/null +++ b/src/CodeIndex/Cli/AtomicFileWriter.cs @@ -0,0 +1,77 @@ +using System.Text; +using System.Text.Json; +using CodeIndex.Indexer; + +namespace CodeIndex.Cli; + +internal static class AtomicFileWriter +{ + public static void WriteText(string path, string contents, Encoding encoding, Action? applyFileMode = null) + { + Write( + path, + stream => + { + using var writer = new StreamWriter(stream, encoding, bufferSize: 1024, leaveOpen: true); + writer.Write(contents); + writer.Flush(); + }, + applyFileMode); + } + + public static void WriteJson(string path, T value, JsonSerializerOptions? options = null, Action? applyFileMode = null) + { + Write(path, stream => JsonSerializer.Serialize(stream, value, options), applyFileMode); + } + + public static void Write(string path, Action writeContents, Action? applyFileMode = null) + { + ArgumentNullException.ThrowIfNull(writeContents); + + var tempPath = BuildTempPath(path); + var ioTempPath = LongPath.EnsureWindowsPrefix(tempPath); + var ioTargetPath = LongPath.EnsureWindowsPrefix(path); + var moved = false; + + try + { + using (var stream = new FileStream(ioTempPath, FileMode.CreateNew, FileAccess.Write, FileShare.None)) + { + applyFileMode?.Invoke(ioTempPath); + writeContents(stream); + stream.Flush(flushToDisk: true); + } + + File.Move(ioTempPath, ioTargetPath, overwrite: true); + moved = true; + applyFileMode?.Invoke(ioTargetPath); + } + catch + { + if (!moved) + TryDelete(ioTempPath); + throw; + } + } + + private static string BuildTempPath(string path) + { + var directory = Path.GetDirectoryName(path); + var fileName = Path.GetFileName(path); + var tempFileName = $".{fileName}.{Guid.NewGuid():N}.tmp"; + return string.IsNullOrEmpty(directory) + ? tempFileName + : Path.Combine(directory, tempFileName); + } + + private static void TryDelete(string path) + { + try + { + File.Delete(path); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + } + } +} diff --git a/src/CodeIndex/Cli/DataDirectorySecurity.cs b/src/CodeIndex/Cli/DataDirectorySecurity.cs index e57fe808b9..02ff5922a2 100644 --- a/src/CodeIndex/Cli/DataDirectorySecurity.cs +++ b/src/CodeIndex/Cli/DataDirectorySecurity.cs @@ -52,13 +52,8 @@ public static void ApplyPrivateFileMode(string path) public static void WritePrivateText(string path, string contents, Encoding? encoding = null) { - var ioPath = LongPath.EnsureWindowsPrefix(path); - using var stream = File.Open(ioPath, FileMode.OpenOrCreate, FileAccess.Write, FileShare.Read); - ApplyPrivateFileMode(ioPath); - stream.SetLength(0); var outputEncoding = encoding is null || encoding.CodePage == Encoding.UTF8.CodePage ? Utf8NoBom : encoding; - using var writer = new StreamWriter(stream, outputEncoding); - writer.Write(contents); + AtomicFileWriter.WriteText(path, contents, outputEncoding, ApplyPrivateFileMode); } public static string? ReadTextWithinLimit(string path, int maxBytes, FileShare share = FileShare.Read) diff --git a/src/CodeIndex/Cli/IndexCommandRunner.FullScan.cs b/src/CodeIndex/Cli/IndexCommandRunner.FullScan.cs index 01346ef5c2..07ea27cbd4 100644 --- a/src/CodeIndex/Cli/IndexCommandRunner.FullScan.cs +++ b/src/CodeIndex/Cli/IndexCommandRunner.FullScan.cs @@ -333,7 +333,7 @@ private static void SaveScanCheckpoint(string path, string? currentHead, IReadOn .Where(directory => directory.Length > 0) .OrderBy(directory => directory, StringComparer.Ordinal) .ToList()); - File.WriteAllText(path, JsonSerializer.Serialize(checkpoint, new JsonSerializerOptions { WriteIndented = true })); + AtomicFileWriter.WriteJson(path, checkpoint, new JsonSerializerOptions { WriteIndented = true }); } catch (IOException) { diff --git a/src/CodeIndex/Cli/SuggestionStore.cs b/src/CodeIndex/Cli/SuggestionStore.cs index c7e5d0b28e..f1ee64d99b 100644 --- a/src/CodeIndex/Cli/SuggestionStore.cs +++ b/src/CodeIndex/Cli/SuggestionStore.cs @@ -728,27 +728,7 @@ private void SaveUnlocked(List records) Directory.CreateDirectory(dir); NormalizeRecordDefaults(records); - - var tempPath = _filePath + ".tmp"; - try - { - using (var stream = new FileStream( - tempPath, - FileMode.Create, - FileAccess.Write, - FileShare.None)) - { - JsonSerializer.Serialize(stream, records, s_jsonOptions); - stream.Flush(flushToDisk: true); - } - - File.Move(tempPath, _filePath, overwrite: true); - } - catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) - { - try { File.Delete(tempPath); } catch (Exception deleteEx) when (deleteEx is IOException or UnauthorizedAccessException) { /* best-effort cleanup / ベストエフォートのクリーンアップ */ } - throw; - } + AtomicFileWriter.WriteJson(_filePath, records, s_jsonOptions); } private static bool HasUpstreamSubmission(SuggestionRecord record) => diff --git a/src/CodeIndex/Cli/UpdateChecker.cs b/src/CodeIndex/Cli/UpdateChecker.cs index 68f2e9e603..952e621ad1 100644 --- a/src/CodeIndex/Cli/UpdateChecker.cs +++ b/src/CodeIndex/Cli/UpdateChecker.cs @@ -219,7 +219,7 @@ private static void TryWriteCache(string cachePath, UpdateCheckCache cache) checked_at = cache.CheckedAt.UtcDateTime.ToString("O", CultureInfo.InvariantCulture), latest_tag = cache.LatestTag, }; - File.WriteAllText(cachePath, JsonSerializer.Serialize(payload)); + AtomicFileWriter.WriteJson(cachePath, payload); } catch { diff --git a/tests/CodeIndex.Tests/DataDirectorySecurityTests.cs b/tests/CodeIndex.Tests/DataDirectorySecurityTests.cs index 0c9de4cb47..dac1dec0c3 100644 --- a/tests/CodeIndex.Tests/DataDirectorySecurityTests.cs +++ b/tests/CodeIndex.Tests/DataDirectorySecurityTests.cs @@ -86,6 +86,29 @@ public void WritePrivateText_OnPosix_Forces0600Mode() } } + [Fact] + public void WritePrivateText_MoveFailure_DoesNotLeaveTempFile() + { + var root = Path.Combine(Path.GetTempPath(), $"cdidx_sensitive_file_atomic_{Guid.NewGuid():N}"); + var path = Path.Combine(root, "metadata.info"); + try + { + Directory.CreateDirectory(path); + + var ex = Record.Exception(() => DataDirectorySecurity.WritePrivateText(path, "secret")); + + Assert.NotNull(ex); + Assert.DoesNotContain( + Directory.EnumerateFiles(root), + file => Path.GetFileName(file).EndsWith(".tmp", StringComparison.Ordinal)); + } + finally + { + if (Directory.Exists(root)) + Directory.Delete(root, recursive: true); + } + } + [Fact] public void ReadTextWithinLimit_WhenFileExceedsLimit_ReturnsNull() { diff --git a/tests/CodeIndex.Tests/SuggestionStoreTests.cs b/tests/CodeIndex.Tests/SuggestionStoreTests.cs index d82ddeb5d2..2ae6760e19 100644 --- a/tests/CodeIndex.Tests/SuggestionStoreTests.cs +++ b/tests/CodeIndex.Tests/SuggestionStoreTests.cs @@ -855,14 +855,15 @@ public void TryAdd_MoveFailure_DoesNotLeaveOrphanTmpFile() // 一時ファイルへの書き込みは成功するが、ディレクトリに対する rename は失敗するため、 // `.cdidx/` に孤児が蓄積しないよう一時ファイルがクリーンアップされる必要がある (#1574)。 var filePath = Path.Combine(_tempDir, "suggestions-codeindex.json"); - var tmpPath = filePath + ".tmp"; Directory.CreateDirectory(filePath); var record = MakeRecord("other", null, "Move failure cleanup"); var ex = Record.Exception(() => _store.TryAdd(record)); Assert.NotNull(ex); - Assert.False(File.Exists(tmpPath), $"Orphan .tmp file should be cleaned up after Move failure: {tmpPath}"); + Assert.DoesNotContain( + Directory.EnumerateFiles(_tempDir), + file => Path.GetFileName(file).EndsWith(".tmp", StringComparison.Ordinal)); } // --- Helpers / ヘルパー ---