diff --git a/lib/src/platform/io/model_download_manager_io.dart b/lib/src/platform/io/model_download_manager_io.dart index 1376513ce..c5eb430ba 100644 --- a/lib/src/platform/io/model_download_manager_io.dart +++ b/lib/src/platform/io/model_download_manager_io.dart @@ -798,6 +798,10 @@ class DefaultModelDownloadManager implements ModelDownloadManager { } } + // A path whose resolution is denied (errno 5) and whose type also reads as + // `notFound` (for example a parent ACL denial) is walked past like a missing + // one. The operation then fails later, as an access denial on create or + // write, or a cache miss for cacheOnly; it never reports success. String _cacheLockKey(Directory cacheDirectory) { var ancestor = path.normalize(path.absolute(cacheDirectory.path)); final missing = []; @@ -1861,7 +1865,8 @@ enum _Removal { removed, inUse, skipped } /// with `ERROR_ACCESS_DENIED`, and so does the attribute query behind /// [FileSystemEntity.type], which reports that failure as /// [FileSystemEntityType.notFound]; callers tell it from a real denial by -/// that type. +/// that type. A different ACL denial can read as `notFound` too; see +/// `_cacheLockKey` for what happens then. bool _mayBeWindowsDeletePending(FileSystemException error) => Platform.isWindows && error.osError?.errorCode == 5; diff --git a/test/unit/platform/io/model_download_manager_io_concurrency_test.dart b/test/unit/platform/io/model_download_manager_io_concurrency_test.dart index 99022b5ed..b2930f12c 100644 --- a/test/unit/platform/io/model_download_manager_io_concurrency_test.dart +++ b/test/unit/platform/io/model_download_manager_io_concurrency_test.dart @@ -35,7 +35,10 @@ void main() { }); setUp(() async { - root = await Directory.systemTemp.createTemp('llamadart_cache_lock_'); + final created = await Directory.systemTemp.createTemp( + 'llamadart_cache_lock_', + ); + root = Directory(await created.resolveSymbolicLinks()); addTearDown(() => root.delete(recursive: true)); server = await _HoldingServer.start(); addTearDown(server.close); diff --git a/test/unit/platform/io/model_download_manager_io_removal_test.dart b/test/unit/platform/io/model_download_manager_io_removal_test.dart index 8f866ee6d..912edffc9 100644 --- a/test/unit/platform/io/model_download_manager_io_removal_test.dart +++ b/test/unit/platform/io/model_download_manager_io_removal_test.dart @@ -136,13 +136,15 @@ void main() { return finishDelete; } - Future clear() => - IOOverrides.runWithIOOverrides(manager.clear, overrides); + Future withOverrides(Future Function() body) => + IOOverrides.runWithIOOverrides(body, overrides); - /// Starts [clear] and holds it at the removal lock, after it has checked - /// the entry, until the returned function runs. + Future clear() => withOverrides(manager.clear); + + /// Starts [operation] and holds it at the removal lock, after it has + /// checked the entry, until the returned function runs. Future<({Future clearing, void Function() proceed})> - clearHeldAtRemovalLock() async { + heldAtRemovalLock(Future Function() operation) async { final waiting = Completer(); final proceed = Completer(); modelCacheEntryLocker = (lockPath, {required throwIfCancelled}) async { @@ -163,15 +165,24 @@ void main() { } } - final clearing = clear(); + final clearing = withOverrides(operation); addTearDown(() async { release(); await clearing.then((_) {}, onError: (Object _) {}); }); - await waiting.future; + await waiting.future.timeout( + const Duration(seconds: 10), + onTimeout: () => throw StateError( + 'The operation never reached the .removal.lock: it no longer takes ' + 'the lock this test holds it at.', + ), + ); return (clearing: clearing, proceed: release); } + Future<({Future clearing, void Function() proceed})> + clearHeldAtRemovalLock() => heldAtRemovalLock(manager.clear); + void expectOnlyLocksLeftOnceDeleted(void Function() finishDelete) { finishDelete(); expect( @@ -212,6 +223,29 @@ void main() { expectOnlyLocksLeftOnceDeleted(finishDelete); }); + test('is skipped by remove when the deletion happens while it waits for ' + 'the removal lock', () async { + final cacheKey = (await manager.list()).single.cacheKey; + final held = await heldAtRemovalLock(() => manager.remove(cacheKey)); + final finishDelete = markPendingDelete(); + held.proceed(); + + await held.clearing; + expectOnlyLocksLeftOnceDeleted(finishDelete); + }); + + test('is skipped by prune when the deletion happens while it waits for ' + 'the removal lock', () async { + final held = await heldAtRemovalLock( + () => manager.prune(maxAge: Duration.zero), + ); + final finishDelete = markPendingDelete(); + held.proceed(); + + await held.clearing; + expectOnlyLocksLeftOnceDeleted(finishDelete); + }); + test('is not skipped when the entry still reads as a directory: clear ' 'reports the access denial', () async { markPendingDelete();