Prevent crash from ZipFile GC cleanup I/O errors

[Daniel-ADFA]

When a ZipFile is garbage collected and its underlying file descriptor is no longer valid, the FinalizerDaemon throws UncheckedIOException wrapping an EIO error. This is a non-critical platform issue; the object is already unreachable and OS reclaims the fd on process exit. Detect this specific pattern via CleanableResource/PhantomCleanable in the stack trace, report to Sentry, and return without killing the process.

[coderabbitai]

📝 Walkthrough

Release Notes

• Prevent crashes from ZipFile garbage collection I/O errors: Detects when UncheckedIOException is thrown by the FinalizerDaemon during cleanup of unreachable ZipFile objects (identified by CleanableResource or PhantomCleanable in the stack trace). Non-fatal cleanup failures are now logged as warnings instead of terminating the application.

• CachedJarFileSystem close method annotation: Added @Throws(IOException::class) declaration to doClose() method to properly reflect the exception contract, though the implementation already handles IOException gracefully.

Risks & Best Practices Violations

• ⚠️ Fragile exception detection pattern: Uses String.contains() to match class names against "CleanableResource" and "PhantomCleanable". This substring matching approach is brittle and could inadvertently match unrelated exception classes that happen to contain these strings. Consider using fully qualified class name comparison instead.

• ⚠️ Silent exception suppression: While the exception is logged, the application silently continues without reporting to Sentry despite the PR description mentioning Sentry reporting. This may result in untracked cleanup failures and could mask related platform issues.

• ⚠️ Stack trace inspection fragility: The fix depends on internal JDK implementation details (PhantomCleanable, CleanableResource class names) which could change across Java/Android versions, potentially causing the detection to fail silently or catch false positives.

• ⚠️ No fallback if exception is wrapped: If the UncheckedIOException is wrapped in another exception type, the detection will fail and the application will crash despite this being a known non-fatal condition.

Walkthrough

Added non-fatal GC cleanup failure detection to IDEApplication that intercepts UncheckedIOException stack traces containing CleanableResource or PhantomCleanable references, short-circuiting crash handling for these known I/O errors. Also annotated CachedJarFileSystem.doClose() with @Throws(IOException::class) to explicitly declare exception behavior.

Changes

Cohort / File(s)	Summary
GC Cleanup Error Handling app/src/main/java/com/itsaky/androidide/app/IDEApplication.kt	Added private isNonFatalGcCleanupFailure() helper method to detect non-fatal I/O exceptions from GC cleanup routines; uncaught exception handler now checks this condition and returns early if matched, preventing crash reporting for these benign errors.
Exception Declaration subprojects/javac-services/src/main/java/com/itsaky/androidide/javac/services/fs/CachedJarFileSystem.kt	Added @Throws(IOException::class) annotation to doClose() method to explicitly declare the exception can be thrown, matching the existing catch block behavior.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• (ADFA-2539) Fix EIO crash during ZipFile finalization  #852: Addresses non-fatal I/O errors from GC finalization of JAR/Zip resources and includes complementary modifications to CachedJarFileSystem exception handling.

Suggested reviewers

• itsaky-adfa
• jomen-adfa

Poem

🐰 A rabbit hops through cleanup's mess,
Catching GC errors—nothing less!
PhantomCleanables can't make us frown,
We log and hop along the town ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

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

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Prevent crash from ZipFile GC cleanup I/O errors' clearly and concisely summarizes the main change: detecting and handling non-fatal garbage collection cleanup I/O errors to prevent process crashes.
Description check	✅ Passed	The description directly explains the problem (ZipFile GC cleanup throwing UncheckedIOException), the root cause (non-critical platform issue), and the solution (detect pattern and report without crashing), all matching the implemented changes.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch ADFA-2539-fix

---

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 and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@app/src/main/java/com/itsaky/androidide/app/IDEApplication.kt`:
- Around line 187-190: The non-fatal GC cleanup branch in
isNonFatalGcCleanupFailure currently only logs a warning via logger.warn but
does not report to Sentry as promised; modify the branch in IDEApplication (the
block that checks isNonFatalGcCleanupFailure(exception)) to call the Sentry
reporting API (e.g., Sentry.captureException or your app's Sentry wrapper
method) with the exception before returning so the error is both logged and sent
to Sentry. Ensure the capture call happens immediately before the existing
return and uses the same exception object passed into
isNonFatalGcCleanupFailure.

🧹 Nitpick comments (2)

subprojects/javac-services/src/main/java/com/itsaky/androidide/javac/services/fs/CachedJarFileSystem.kt (1)

50-57: @Throws(IOException::class) is contradicted by the internal catch.

doClose() catches and swallows every IOException on line 54, so no IOException will ever propagate to callers. The @Throws annotation generates a throws IOException clause in bytecode, which misleads Java callers into thinking they need to handle it.

Either remove the annotation (if the intent is to always swallow), or re-throw the exception after logging (if the intent is to let callers handle it).

Option A – remove the annotation (current behavior, swallow):

-  `@Throws`(IOException::class)
   fun doClose() {

Option B – propagate after logging (match the annotation):

   `@Throws`(IOException::class)
   fun doClose() {
     try {
       super.close()
     } catch (e: IOException) {
       log.warn("IOException during CachedJarFileSystem close", e)
+      throw e
     }
   }

app/src/main/java/com/itsaky/androidide/app/IDEApplication.kt (1)

200-206: Consider also inspecting the cause's stack trace for robustness.

Depending on the Android/JDK version, the CleanableResource/PhantomCleanable frames may appear in the wrapped IOException's stack trace rather than the UncheckedIOException's own. Additionally, checking that the cause is an IOException would tighten the match.

Suggested hardening

 private fun isNonFatalGcCleanupFailure(exception: Throwable): Boolean {
     if (exception !is java.io.UncheckedIOException) return false
-    return exception.stackTrace.any {
-        it.className.contains("CleanableResource") ||
-            it.className.contains("PhantomCleanable")
-    }
+    val framesToCheck = exception.stackTrace.toList() +
+        (exception.cause?.stackTrace?.toList() ?: emptyList())
+    return framesToCheck.any {
+        it.className.contains("CleanableResource") ||
+            it.className.contains("PhantomCleanable")
+    }
 }