gnodet commented on issue #281: URL: https://github.com/apache/maven-clean-plugin/issues/281#issuecomment-5803630425
Thanks for the detailed report. The regression between 3.3.2 and 3.4.1 is real and the root cause is the switch from `File.delete()` to the NIO `Files.deleteIfExists(Path)` API. On Windows, `Files.deleteIfExists()` on a directory throws an `IOException` (typically `AccessDeniedException` or a Windows-specific error) if any handle is open on that directory — including handles held by the JVM itself (class loading) or by Windows Defender/Search Indexer. The old `File.delete()` would silently return `false` in the same situation, which was less correct but more tolerant. **Does the current code already handle this?** Yes, partially: `retryOnError=true` is the default, and `Cleaner.tryDelete()` already retries up to 3 times (after 50ms, 250ms, 750ms delays) calling `System.gc()` on Windows between attempts to encourage the JVM to release file handles. So the question is why this isn't working for you. It's possible that: - The retry delays are too short relative to when the handle is released - `System.gc()` is not sufficient if the handle is held by a non-GC resource **Does PR #328 help?** PR [#328](https://github.com/apache/maven-clean-plugin/pull/328) introduces a smarter batch retry in `BackgroundCleaner`, but that's only active when `fast=true`. For the non-fast path (#328 doesn't change `Cleaner.tryDelete()`), no direct improvement. **Should `fast=true` become the default?** That's worth considering. The fast path avoids the problem entirely by atomically renaming the directory (a single OS call) and then deleting in the background. The rename is instantaneous and doesn't require all handles to be closed. The constraints for `fast=true` are: the staging dir must be on the same filesystem as `target/`, and the plugin requires Maven 4 API. For multi-module projects this already works well. **Alternative: apply the same batch retry in the non-fast path** Rather than per-file `System.gc()` + sleep, the non-fast `Cleaner` could collect all deletion failures in `postVisitDirectory` / `visitFile`, then do a single batch sleep and retry pass — same strategy as `BackgroundCleaner.deleteInBackground()`. This would be less disruptive than changing the default and would fix the Windows empty-dir case without requiring `fast=true`. I'll leave this open and track it as a potential improvement. -- 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]
