ADFA-3945: Handle reader-thread IOException on tar.xz extraction timeout

[fryanpan]

Jira Ticket: https://appdevforall.atlassian.net/browse/ADFA-3945
Sentry Issue: https://appdevforall-inc-9p.sentry.io/issues/APPDEVFORALL-V0

Reproduction Details

IdeArchiveServiceImpl.extractTarXzViaTermux ran Termux tar and drained the child's stdout on a separate reader thread (thread { process.inputStream.bufferedReader().useLines { … } }). On a 2-minute timeout the code calls destroyForcibly(), which closes the pipe; the reader thread's in-flight read() then throws InterruptedIOException on a bare thread with no handler → uncaught → app crash.

Stack Trace

InterruptedIOException: read interrupted by close() on another thread
  at libcore.io.Linux.read(Linux.java)
  ... (BufferedInputStream / StreamDecoder / BufferedReader.readLine) ...
  at com.itsaky.androidide.plugins.manager.services.IdeArchiveServiceImpl.extractTarXzViaTermux$lambda$0$0$1(Unknown Source:31)
  at kotlin.concurrent.ThreadsKt$thread$thread$1.run(Unknown Source:2)

(thread: tar-xz-extract-output · device.class: high)

User Steps

User steps leading up to crash, based on Sentry breadcrumbs:

• Fires at app startup while a bundled component is being extracted (per related events, the NDK-installer plugin org.appdevforall.ndkinstaller.cgp). The extraction is force-killed (timeout/cancel) and the reader thread dies mid-read. (The crashing event itself has no UI breadcrumbs — it's a background reader thread.)

Was able to reproduce in a unit test?

No.
What remains after this fix is the ProcessBuilder/Termux path — extractTarXzViaTermux is a private fun on a final class and execs a hard-coded on-device tar binary ($TERMUX_BIN_PATH/tar) absent on any JVM test host, so start() can't run there. However, Option B removes the concurrency hazard entirely (no reader thread), so the failure class this ticket is about no longer exists to be triggered. Making the remaining exec path regression-testable would need an injectable process/command seam (@VisibleForTesting) — tracked as a follow-up. The change compiles cleanly (:plugin-manager:compileV8DebugKotlin BUILD SUCCESSFUL).

What Was Fixed

Root-cause fix (Hal's suggestion, "Option B"), not a symptom catch. Redirect the child's combined stdout/stderr to a temp file via ProcessBuilder.redirectOutput(logFile) and read it back after waitFor(), dropping the reader thread entirely:

• With OS-side redirection there is no concurrent read() to be interrupted when destroyForcibly() closes the pipe — the InterruptedIOException-on-a-bare-thread crash cannot occur.
• It also removes the latent full-pipe-buffer deadlock risk (the original reason a drainer thread existed).
• destroyForcibly() on timeout is retained; the temp log is deleted after use.
• Makes a future timeout/cancellation change safe — there's no race to reintroduce.

(Considered and rejected "Option A" — close stdout then destroyForcibly(): on Android, closing a stream under a blocked read is exactly what produces read interrupted by close(), so it wouldn't reliably give a clean EOF and would still need the catch.)

Testing

:plugin-manager:compileV8DebugKotlin → BUILD SUCCESSFUL. No automated runtime test (see above — the remaining exec path isn't JVM-testable; the bug class itself is removed by construction). Branch history was rewritten to a single clean commit implementing Option B.

Follow-up (separate): make the extraction timeout adaptive for slow devices + add progress/cancel UI + a retry path (the user-facing "extraction actually succeeds" improvement), and add an @VisibleForTesting process seam.

---

Fixes APPDEVFORALL-V0

[fryanpan]

⚠️ Heads-up for reviewers: approach changed + history rewritten (force-push).

This PR previously caught the reader-thread IOException (symptom fix). It now implements the root-cause fix (Hal's Option B): redirect tar's output to a temp file via ProcessBuilder.redirectOutput(), drop the reader thread, and read the log after waitFor(). No concurrent read → the InterruptedIOException-on-a-bare-thread crash can't occur; also removes a latent pipe-buffer deadlock. destroyForcibly() on timeout retained.

Rejected Option A (close-then-destroy): on Android, closing a stream under a blocked read is precisely what throws read interrupted by close(), so it wouldn't give a clean EOF.

Branch was rewritten to a single clean commit (1d4c800ee); the description is updated to match. Follow-up (separate): adaptive timeout + progress/retry UX, and an @VisibleForTesting process seam for regression testing.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0dd94c57-c3f9-4e91-8938-6dc3ee20adcd

📥 Commits

Reviewing files that changed from the base of the PR and between b23bedb and cb178fa.

📒 Files selected for processing (1)

• plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeArchiveServiceImpl.kt

🚧 Files skipped from review as they are similar to previous changes (1)

• plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeArchiveServiceImpl.kt

---

📝 Walkthrough

• Fixed the APPDEVFORALL-V0 crash in IdeArchiveServiceImpl.extractTarXzViaTermux by eliminating the background thread that drained Termux tar output; destroyForcibly() on timeout could previously close a pipe while a reader thread was blocked in read(), leading to an uncaught InterruptedIOException.
• Changed Termux tar execution to redirect combined stdout/stderr to a temporary log file via ProcessBuilder.redirectOutput(logFile) and read the log synchronously after waitFor().
• Kept the existing timeout behavior (2-minute wait, then destroyForcibly()); added a bounded waitFor after the forcible destroy to ensure the process is gone before reading exit code/log.
• Removed the pipe-buffer/deadlock risk associated with concurrent pipe draining by avoiding in-flight pipe reads entirely.
• Added KDoc documenting the log-file redirection rationale and the specific concurrency hazard addressed.
• Risk / best-practice: still depends on the external on-device Termux tar binary and on reliable temp-file cleanup; temp log deletion failure is logged as a warning.
• Risk / best-practice: no automated runtime regression test was added for the Termux ProcessBuilder path; validation remains primarily device/runtime based.

Walkthrough

extractTarXzViaTermux now captures Termux tar output in a temporary log file, handles timeout and interruption outcomes, reads the log after execution, and deletes it during cleanup.

Changes

Termux tar extraction output redirection

Layer / File(s)	Summary
File-redirection execution and cleanup plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeArchiveServiceImpl.kt	Removes background stream draining, documents the revised flow, redirects combined output to a temporary log file, enforces timeout handling, preserves interruptions, logs captured output, and cleans up the file.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: Daniel-ADFA

Poem

🐇 Tar hops through a log-file trail,
Timeout guards its leafy veil.
Threads rest softly in their bed,
Then cleanup clears the path ahead.
Hop hop, the archive is read!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title matches the main fix: handling the tar.xz timeout crash from the reader thread IOException.
Description check	✅ Passed	The description directly describes the crash, root cause, fix, and testing status for this extraction timeout change.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch ADFA-3945-tarxz-reader-thread-ioexception

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeArchiveServiceImpl.kt`:
- Around line 293-295: The process lifecycle in IdeArchiveServiceImpl’s tar
execution block is incomplete: destroyForcibly() is asynchronous, and
InterruptedException from waitFor(...) can leave the spawned process running.
Update the tar handling around process.waitFor(...) so that when completion
times out or an interruption occurs, the process is forcibly terminated and then
waited on before continuing, and make sure the interruption path in this method
cleans up the process and preserves the interrupted state.
- Around line 312-313: The temporary extraction log cleanup in
IdeArchiveServiceImpl is ignoring deletion failures, so the log can remain
behind without notice. Update the cleanup path around the .also block after
extraction to check the result of logFile.delete(), and if it fails, record a
warning or error with the existing logger so the failure is visible. Use the
logFile cleanup point in this extraction flow to keep the behavior clear and
localized.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 93a1874f-bc7d-4869-b931-45ba99a7294c

📥 Commits

Reviewing files that changed from the base of the PR and between 1d4c800 and b23bedb.

📒 Files selected for processing (1)

• plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeArchiveServiceImpl.kt