Skip to content

Commit 5216bcb

Browse files
authored
Fix cache-entry remove on Windows. (#106)
1 parent 0df45c9 commit 5216bcb

2 files changed

Lines changed: 56 additions & 3 deletions

File tree

src/cache.toit

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,15 @@ class Cache:
7676
if file.is-file key-path:
7777
file.delete key-path
7878
else if file.is-directory key-path:
79-
tmp-path := directory.mkdtemp key-path
80-
file.rename key-path tmp-path
81-
directory.rmdir --recursive --force tmp-path
79+
// Move the entry into a freshly created temp directory so concurrent
80+
// readers see the entry either fully present or gone. We rename it
81+
// *into* the temp dir (rather than *over* it) so the destination
82+
// child path is guaranteed not to exist: this works on Windows,
83+
// where 'rename' cannot overwrite an existing directory, and avoids
84+
// a race window with any sibling using the same temp name.
85+
tmp-dir := directory.mkdtemp key-path
86+
file.rename key-path (fs.join tmp-dir "entry")
87+
directory.rmdir --recursive --force tmp-dir
8288

8389
/**
8490
Whether the cache contains the given $key.

tests/cache_test.toit

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import expect show *
1111
main:
1212
test-file-cache
1313
test-dir-cache
14+
test-remove
1415

1516
test-file-cache:
1617
cache-dir := directory.mkdtemp "/tmp/cache_test-"
@@ -326,3 +327,49 @@ test-dir-cache:
326327

327328
finally:
328329
directory.rmdir --recursive cache-dir
330+
331+
test-remove:
332+
cache-dir := directory.mkdtemp "/tmp/cache_test-"
333+
try:
334+
c := cache.Cache --app-name="test" --path=cache-dir
335+
336+
// Removing a non-existing entry is a no-op.
337+
c.remove "missing"
338+
339+
// Remove a file entry.
340+
file-key := "file-key"
341+
c.get file-key: | store/cache.FileStore | store.save #[1, 2, 3]
342+
expect (c.contains file-key)
343+
c.remove file-key
344+
expect-not (c.contains file-key)
345+
346+
// Remove a directory entry.
347+
dir-key := "dir-key"
348+
c.get-directory-path dir-key: | store/cache.DirectoryStore |
349+
store.with-tmp-directory: | dir |
350+
write-content --path="$dir/file" --content=#[4, 5, 6]
351+
store.move dir
352+
expect (c.contains dir-key)
353+
c.remove dir-key
354+
expect-not (c.contains dir-key)
355+
356+
// Remove a nested directory entry, then recreate it.
357+
nested-key := "nested/dir-key"
358+
c.get-directory-path nested-key: | store/cache.DirectoryStore |
359+
store.with-tmp-directory: | dir |
360+
write-content --path="$dir/file" --content=#[7, 8, 9]
361+
store.move dir
362+
expect (c.contains nested-key)
363+
c.remove nested-key
364+
expect-not (c.contains nested-key)
365+
366+
c.get-directory-path nested-key: | store/cache.DirectoryStore |
367+
store.with-tmp-directory: | dir |
368+
write-content --path="$dir/file" --content=#[10, 11, 12]
369+
store.move dir
370+
expect (c.contains nested-key)
371+
nested-path := c.get-directory-path nested-key
372+
expect-equals #[10, 11, 12] (file.read-contents "$nested-path/file")
373+
374+
finally:
375+
directory.rmdir --recursive cache-dir

0 commit comments

Comments
 (0)