tballison commented on PR #3532:
URL: https://github.com/apache/tika/pull/3532#issuecomment-6641432933
Thank you, @dpol1 .
My bot only has two nits... not clear to me if these are real? Let me know
what you think.
```
1. Extra pid-reuse check. Java's ProcessHandle.destroyForcibly() already
compares the start time before killing, I
believe. If so, the Descendant record, the second start-time lookup and
the second isAlive() check can all go:
- Simpler version: descendants = handle.descendants().toList();
process.destroyForcibly();
descendants.forEach(ProcessHandle::destroyForcibly).
- Side effects of the current check: a descendant with no startInstant
is never killed.
- If the parent exits during the walk, all its descendants are dropped,
and those are exactly the orphans this PR
is meant to catch.
- The local JDK has no src.zip, so I couldn't confirm the JDK guard;
worth checking before asking for the change.
2. Missed call site. checkCommandWithTimeout (ProcessUtils.java:498) still
uses plain destroyForcibly(). The ticket
says "wherever ProcessUtils kills a process". It only runs short
version checks, so either convert it or leave it
out on purpose.
```
--
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]
