[
https://issues.apache.org/jira/browse/TIKA-5040?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18138217#comment-18138217
]
ASF GitHub Bot commented on TIKA-5040:
--------------------------------------
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.
```
> Orphaned external processes after a forked server is killed or ProcessUtils
> stops waiting
> -----------------------------------------------------------------------------------------
>
> Key: TIKA-5040
> URL: https://issues.apache.org/jira/browse/TIKA-5040
> Project: Tika
> Issue Type: Bug
> Components: tika-core, tika-pipes
> Affects Versions: 4.2.0
> Reporter: Davide Polato
> Priority: Minor
> Fix For: 4.3.0
>
>
> In looking at how to stop a running parse (TIKA-4795), I found that killing a
> forked pipes server leaves its external processes running.
> PerClientServerManager and SharedServerManager kill the server with
> destroyForcibly, which is SIGKILL on Linux. ProcessUtils kills the processes
> it started in execute()'s finally block or in a shutdown hook. SIGKILL ends
> the JVM before either runs.
> When the server exits on its own, the hook runs, but it kills only the direct
> process. With {{sh -c 'sleep 300; true'}}, the sleep keeps running. The
> timeout and interrupt paths of ProcessUtils.execute have the same gap.
> TIKA-3103 shows what this costs: orphaned tesseract processes at 100% CPU for
> over ten minutes.
> I have tests that reproduce each case on main. I'll open a PR that also kills
> the descendants, via ProcessHandle.descendants(), in both server managers and
> wherever ProcessUtils kills a process.
> TesseractOCRParser uses a separate hook in AbstractExternalProcessParser. I'd
> leave that for a follow-up, unless you'd rather have it in this PR?
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
