gnodet opened a new pull request, #345:
URL: https://github.com/apache/maven-clean-plugin/pull/345
## Summary
Replaces the per-file `System.gc()` + sleep retry loop in the non-fast
`Cleaner` path with the same batch retry strategy already used by
`BackgroundCleaner` (introduced for the fast path in MCLEAN-102 / PR #328).
Closes #281. Helps #285.
## Root Cause
The non-fast deletion path (`fast=false`, the default) in
`Cleaner.tryDelete()` retried failed deletions per-file, calling `System.gc()`
on Windows between attempts and sleeping 50ms/250ms/750ms per file. On Windows,
files can be temporarily locked by virus scanners or search indexers, and
`System.gc()` was meant to "release JVM-held file handles." However:
- `System.gc()` is a stop-the-world hint — unreliable and expensive.
- Per-file sleeps multiply linearly with the number of files.
- This is the exact anti-pattern MCLEAN-102 eliminated from the fast path.
Issues #281 and #285 both report Windows deletion failures in this code path.
## Fix
**`Cleaner.java`:**
1. **Walk phase** — `tryDelete()` makes one deletion attempt. On failure:
- If `force=true` and failure is `AccessDeniedException`: try
`setWritable()` and retry immediately (permissions fix, not timing — unchanged
behavior).
- Otherwise: add the path to `retryQueue` and return `true`
(tentatively). Returning `true` means the parent directory is still attempted
for deletion at walk time and also queued if it fails. No `System.gc()`, no
sleep.
2. **Batch retry phase** — After `Files.walkFileTree()` completes, if
`retryQueue` is non-empty:
- Sleep once (`BATCH_RETRY_DELAY_MS = 250ms` — same as
`BackgroundCleaner`).
- Retry each path in walk order (files before their containing directory
— natural walk order guarantees correct bottom-up deletion).
- Log a warning for any path that still cannot be deleted.
3. **Constant** — `Cleaner.BATCH_RETRY_DELAY_MS = 250` (package-visible,
referenced by `BackgroundCleaner.deleteSilently`'s Javadoc).
**`BackgroundCleaner.java`:**
`deleteSilently()` now calls `new Cleaner(this).delete(dir)` instead of
using the cleaner as a raw `FileVisitor` via `Files.walkFileTree`. This ensures
the batch retry also applies when the background thread processes directories
(the copy constructor propagates `retryOnError`).
## Tests
Two new tests in `CleanerTest`:
- `batchRetrySucceedsWhenFileBecomesAvailableDuringDelay`: simulates a
transiently locked file (non-writable parent dir restored by a background
thread during the batch-retry sleep). Verifies no warning is logged and all
files are deleted.
- `batchRetryLogsWarningAfterRetryWhenStillFailing`: permanently
non-writable parent. Verifies that warnings are logged after the retry phase
(not during the walk), and the file remains.
## Related
- #281 — Windows: `maven-clean-plugin` fails to delete directory (non-fast
path)
- #285 — Random deletion error on Azure DevOps Pipeline
- PR #328 / MCLEAN-102 — Same fix for the fast path (`BackgroundCleaner`)
---
*Co-authored-by: Hermes Agent (Claude Sonnet 4.6)*
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
