From c5a1de09f42d17491010d5331334f420fa056a3d Mon Sep 17 00:00:00 2001 From: Florian Loitsch Date: Thu, 14 May 2026 15:56:43 +0200 Subject: [PATCH] Fix cache-entry remove on Windows. --- src/cache.toit | 12 ++++++++--- tests/cache_test.toit | 47 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/src/cache.toit b/src/cache.toit index 969c90e..ea94e06 100644 --- a/src/cache.toit +++ b/src/cache.toit @@ -76,9 +76,15 @@ class Cache: if file.is-file key-path: file.delete key-path else if file.is-directory key-path: - tmp-path := directory.mkdtemp key-path - file.rename key-path tmp-path - directory.rmdir --recursive --force tmp-path + // Move the entry into a freshly created temp directory so concurrent + // readers see the entry either fully present or gone. We rename it + // *into* the temp dir (rather than *over* it) so the destination + // child path is guaranteed not to exist: this works on Windows, + // where 'rename' cannot overwrite an existing directory, and avoids + // a race window with any sibling using the same temp name. + tmp-dir := directory.mkdtemp key-path + file.rename key-path (fs.join tmp-dir "entry") + directory.rmdir --recursive --force tmp-dir /** Whether the cache contains the given $key. diff --git a/tests/cache_test.toit b/tests/cache_test.toit index 61480e7..a880035 100644 --- a/tests/cache_test.toit +++ b/tests/cache_test.toit @@ -11,6 +11,7 @@ import expect show * main: test-file-cache test-dir-cache + test-remove test-file-cache: cache-dir := directory.mkdtemp "/tmp/cache_test-" @@ -326,3 +327,49 @@ test-dir-cache: finally: directory.rmdir --recursive cache-dir + +test-remove: + cache-dir := directory.mkdtemp "/tmp/cache_test-" + try: + c := cache.Cache --app-name="test" --path=cache-dir + + // Removing a non-existing entry is a no-op. + c.remove "missing" + + // Remove a file entry. + file-key := "file-key" + c.get file-key: | store/cache.FileStore | store.save #[1, 2, 3] + expect (c.contains file-key) + c.remove file-key + expect-not (c.contains file-key) + + // Remove a directory entry. + dir-key := "dir-key" + c.get-directory-path dir-key: | store/cache.DirectoryStore | + store.with-tmp-directory: | dir | + write-content --path="$dir/file" --content=#[4, 5, 6] + store.move dir + expect (c.contains dir-key) + c.remove dir-key + expect-not (c.contains dir-key) + + // Remove a nested directory entry, then recreate it. + nested-key := "nested/dir-key" + c.get-directory-path nested-key: | store/cache.DirectoryStore | + store.with-tmp-directory: | dir | + write-content --path="$dir/file" --content=#[7, 8, 9] + store.move dir + expect (c.contains nested-key) + c.remove nested-key + expect-not (c.contains nested-key) + + c.get-directory-path nested-key: | store/cache.DirectoryStore | + store.with-tmp-directory: | dir | + write-content --path="$dir/file" --content=#[10, 11, 12] + store.move dir + expect (c.contains nested-key) + nested-path := c.get-directory-path nested-key + expect-equals #[10, 11, 12] (file.read-contents "$nested-path/file") + + finally: + directory.rmdir --recursive cache-dir