Copilot commented on code in PR #2717:
URL: https://github.com/apache/tika/pull/2717#discussion_r3006189191
##########
tika-core/src/main/java/org/apache/tika/fork/ForkClient.java:
##########
@@ -324,6 +324,7 @@ public synchronized void close() {
}
if (jar != null) {
jar.delete();
+ jar.deleteOnExit();
Review Comment:
`File#deleteOnExit()` can throw `SecurityException`. Since `close()`
currently doesn’t declare/expect to throw and is used in failure-cleanup paths,
this new call can cause unexpected runtime failure during cleanup. Please guard
the `deleteOnExit()` call (and any conditional logic around it) to keep
`close()` best-effort.
```suggestion
try {
jar.deleteOnExit();
} catch (SecurityException ignore) {
// best-effort cleanup; ignore inability to register
deleteOnExit
}
```
##########
tika-core/src/main/java/org/apache/tika/fork/ForkClient.java:
##########
@@ -324,6 +324,7 @@ public synchronized void close() {
}
if (jar != null) {
jar.delete();
+ jar.deleteOnExit();
Review Comment:
`deleteOnExit()` is currently registered unconditionally. Even when
`jar.delete()` succeeds, `deleteOnExit()` still adds the path to the JVM’s
global delete-on-exit list, which can grow over time in long-running processes
and provides no benefit after a successful delete. Consider checking the
boolean result of `jar.delete()` (or using `Files.delete`/`deleteIfExists`) and
only calling `deleteOnExit()` when the immediate deletion fails (optionally log
that failure, similar to `TemporaryResources#createTempFile`).
```suggestion
if (!jar.delete()) {
jar.deleteOnExit();
}
```
--
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]
