Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
fryanpan wants to merge 2 commits into
base: stage
Choose a base branch
from
Open

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

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
847c79e
ADFA-3945: extract tar.xz via redirected output file (removes reader-…
fryanpan Jun 19, 2026
b23bedb
Merge branch 'stage' into ADFA-3945-tarxz-reader-thread-ioexception
hal-eisen-adfa Jun 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 23 additions & 10 deletions ...r/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeArchiveServiceImpl.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import java.io.FilterInputStream
import java.io.InputStream
import java.io.OutputStream
import java.util.concurrent.TimeUnit
import kotlin.concurrent.thread
import kotlin.system.measureTimeMillis

class IdeArchiveServiceImpl(
Expand Down Expand Up @@ -255,6 +254,21 @@ class IdeArchiveServiceImpl(
}
}

/**
* Extracts a `.tar.xz` archive by invoking Termux `tar`.
*
* The child process's combined stdout/stderr is redirected to a temporary log file
* ([ProcessBuilder.redirectOutput]) and read back after the process exits, rather than being
* drained by a separate reader thread. This removes the concurrency hazard behind ADFA-3945
* (Sentry APPDEVFORALL-V0): when a timeout triggers [Process.destroyForcibly] the pipe is
* closed, and a thread blocked in `read()` on it would throw an `InterruptedIOException` on a
* bare thread with no handler, crashing the app. With OS-side redirection there is no in-flight
* read to interrupt, and the process also cannot deadlock on a full pipe buffer.
*
* @param archiveFile the `.tar.xz` to extract.
* @param outputDir the directory to extract into (must exist and be writable).
* @return `true` only if `tar` completed within the timeout with exit code 0.
*/
private fun extractTarXzViaTermux(archiveFile: File, outputDir: File): Boolean {
if (!archiveFile.exists()) {
logger.debug("Archive not found: ${archiveFile.absolutePath}")
Expand All @@ -263,28 +277,25 @@ class IdeArchiveServiceImpl(

logger.debug("Starting Termux tar extraction: ${archiveFile.absolutePath}")

val logFile = File.createTempFile("tarxz-extract", ".log", outputDir)
return runCatching {
val output = StringBuilder()
var exitCode = -1

val elapsed = measureTimeMillis {
val process = ProcessBuilder(
"$TERMUX_BIN_PATH/tar", "-xJf", archiveFile.absolutePath,
"-C", outputDir.canonicalPath, "--no-same-owner"
).redirectErrorStream(true).apply {
environment()["PATH"] = TERMUX_BIN_PATH
}.start()

val reader = thread(name = "tar-xz-extract-output") {
process.inputStream.bufferedReader().useLines { it.forEach(output::appendLine) }
}
).redirectErrorStream(true)
.redirectOutput(logFile)
.apply { environment()["PATH"] = TERMUX_BIN_PATH }
.start()

val completed = process.waitFor(2, TimeUnit.MINUTES)
if (!completed) process.destroyForcibly()
reader.join()
exitCode = if (completed) process.exitValue() else -1
Comment on lines 293 to 295

@coderabbitai coderabbitai Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Wait for the forcibly killed tar process and clean it up on interruption.

destroyForcibly() is asynchronous, so the method can return/read/delete the log while tar is still exiting. Also, if waitFor(...) throws InterruptedException, the started process is never destroyed.

Suggested lifecycle handling 
-                val completed = process.waitFor(2, TimeUnit.MINUTES)
-                if (!completed) process.destroyForcibly()
-                exitCode = if (completed) process.exitValue() else -1
+                try {
+                    val completed = process.waitFor(2, TimeUnit.MINUTES)
+                    if (completed) {
+                        exitCode = process.exitValue()
+                    } else {
+                        process.destroyForcibly()
+                        process.waitFor(10, TimeUnit.SECONDS)
+                        exitCode = -1
+                    }
+                } catch (e: InterruptedException) {
+                    process.destroyForcibly()
+                    Thread.currentThread().interrupt()
+                    throw e
+                }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
val completed = process.waitFor(2, TimeUnit.MINUTES)
if (!completed) process.destroyForcibly()
reader.join()
exitCode = if (completed) process.exitValue() else -1
try {
val completed = process.waitFor(2, TimeUnit.MINUTES)
if (completed) {
exitCode = process.exitValue()
} else {
process.destroyForcibly()
process.waitFor(10, TimeUnit.SECONDS)
exitCode = -1
}
} catch (e: InterruptedException) {
process.destroyForcibly()
Thread.currentThread().interrupt()
throw e
}
🤖 Prompt for 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.

In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeArchiveServiceImpl.kt`
around lines 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.

}

val output = runCatching { logFile.readText() }.getOrDefault("")
when (exitCode) {
0 -> {
logger.debug("Extraction succeeded in ${elapsed}ms: $output")
Expand All @@ -298,6 +309,8 @@ class IdeArchiveServiceImpl(
}.getOrElse { e ->
logger.error("Termux process error: ${e.message}")
false
}.also {
logFile.delete()
Comment on lines +312 to +313

@coderabbitai coderabbitai Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Handle failed deletion of the temporary extraction log.

The log contains combined process output and currently remains silently if deletion fails.

Suggested cleanup hardening 
-        }.also {
-            logFile.delete()
+        }.also {
+            if (logFile.exists() && !logFile.delete()) {
+                logger.warn("Failed to delete Termux extraction log: ${logFile.absolutePath}")
+                logFile.deleteOnExit()
+            }
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
}.also {
logFile.delete()
}.also {
if (logFile.exists() && !logFile.delete()) {
logger.warn("Failed to delete Termux extraction log: ${logFile.absolutePath}")
logFile.deleteOnExit()
}
🤖 Prompt for 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.

In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeArchiveServiceImpl.kt`
around lines 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.

}
}

Expand Down
Loading