ADFA-2794 Another attempt to fix all the TimeoutException / unknown finalizer crashes#1239
Merged
hal-eisen-adfa merged 2 commits intoApr 24, 2026
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR centralizes resource cleanup and finalizer handling: it suppresses FinalizerWatchdogDaemon TimeoutExceptions in the uncaught-exception handler, removes I/O from a finalizer, converts several streams/cursors to scoped resource usage, and ensures vector streams are closed in finally blocks. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
vectormaster/src/main/java/org/appdevforall/codeonthego/vectormaster/VectorMasterView.java (1)
99-117:⚠️ Potential issue | 🟡 MinorSame early-return leak:
vectorStreamnot closed on parser-setup failure.Mirror of the issue in
VectorMasterDrawable.buildVectorModel(). IfXmlPullParserExceptionis thrown at Line 104 duringxpp.setInput(...), the method returns at Line 109 before reaching thetryat Line 125, so the newfinallyat Line 613 never runs and theFileInputStreamopened at Line 92 remains open until finalization — which is the scenario this PR aims to eliminate.🛠️ Close the stream on the early-return path
XmlPullParser xpp; if (vectorStream != null) { try { XmlPullParserFactory parserFactory = XmlPullParserFactory.newInstance(); parserFactory.setNamespaceAware(true); xpp = parserFactory.newPullParser(); xpp.setInput(vectorStream, "utf-8"); } catch (XmlPullParserException xml) { Log.e(TAG, "Error from reading vector image"); xml.printStackTrace(); vectorModel = null; + try { + vectorStream.close(); + } catch (IOException ignored) { + } + vectorStream = null; return; }Or restructure so a single outer
try/finallycovers both parser setup and the parse loop.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vectormaster/src/main/java/org/appdevforall/codeonthego/vectormaster/VectorMasterView.java` around lines 99 - 117, In VectorMasterView (method where vectorStream is used) the XmlPullParserException path can return before the outer finally that closes vectorStream, leaking the FileInputStream; update the catch block around XmlPullParserFactory/newPullParser()/xpp.setInput(...) to explicitly close vectorStream (or refactor so a single outer try/finally encloses both parser setup and parsing) before setting vectorModel = null and returning, ensuring vectorStream is always closed even on parser-setup failures.vectormaster/src/main/java/org/appdevforall/codeonthego/vectormaster/VectorMasterDrawable.java (1)
117-135:⚠️ Potential issue | 🟡 Minor
vectorStreamstill leaks on parser-setup failure.The new
finallyat Line 403 only runs after the main parse looptryat Line 144. WhenXmlPullParserExceptionis thrown while constructing/configuring the parser (Line 123), the method returns at Line 127 without ever closingvectorStream, and also without closing it whenresID != -1/ else paths reassignxpp(although those don't own the stream). This leaves theFileInputStreamopened at Line 110 for the finalizer — the exact situation this PR is trying to avoid.🛠️ Close the stream on the early-return path
XmlPullParser xpp; if (vectorStream != null) { try { XmlPullParserFactory parserFactory = XmlPullParserFactory.newInstance(); parserFactory.setNamespaceAware(true); xpp = parserFactory.newPullParser(); xpp.setInput(vectorStream, "utf-8"); } catch (XmlPullParserException xml) { Log.e(TAG, "Error from reading vector image"); xml.printStackTrace(); vectorModel = null; + try { + vectorStream.close(); + } catch (IOException ignored) { + } + vectorStream = null; return; }Alternatively, restructure so a single outer
try/finally(or try-with-resources over a localInputStream) covers both parser setup and the parse loop.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vectormaster/src/main/java/org/appdevforall/codeonthego/vectormaster/VectorMasterDrawable.java` around lines 117 - 135, The parser-setup path can return early and leak the FileInputStream (vectorStream); ensure vectorStream is closed on all early-return paths by either wrapping the InputStream in a try-with-resources or creating a single outer try/finally that closes vectorStream, and then move XmlPullParserFactory/newPullParser setup and the main parse loop (xpp usage) inside that scope; specifically update the code handling vectorStream, XmlPullParserFactory, xpp, and the early returns that set vectorModel to null so that vectorStream.close() always runs (or is auto-closed) before returning, while leaving resources.getXml(resID) behavior unchanged.
🧹 Nitpick comments (1)
app/src/main/java/com/itsaky/androidide/app/IDEApplication.kt (1)
214-218: Consider rate-limiting or sampling the Sentry report to avoid alert storms.
FinalizerWatchdogDaemontimeouts can fire repeatedly once the device's storage is slow (the watchdog is re-armed after each timeout). Reporting every one to Sentry may flood the project with duplicates of the same underlying condition and make the signal harder to read — which is the same concern that led the siblingisNonFatalGcCleanupFailurebranch right above to log-only. Since the goal here is to suppress the crash (not treat it as an actionable bug), consider either:
- Capturing only once per process (guarded by an
AtomicBoolean), or- Attaching a fingerprint / tag (e.g.
fingerprint = ["finalizer-watchdog-timeout"]) and letting Sentry group them, or- Using
Sentry.captureMessage(...)atWARNINGlevel instead ofcaptureException, so stack-trace-based grouping doesn't split events across many issues.Based on learnings (PR 957, IDEApplication.kt): the team previously chose to log-only for
isNonFatalGcCleanupFailureto avoid reopening tickets for non-fatal errors — worth considering whether the same rationale applies here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/itsaky/androidide/app/IDEApplication.kt` around lines 214 - 218, The FinalizerWatchdogDaemon branch currently calls Sentry.captureException for every occurrence (inside isFinalizerWatchdogTimeout), which can flood Sentry; change this to a rate-limited or grouped report: for example, add an AtomicBoolean (e.g., finalizerWatchdogReported) and only call Sentry.captureException once per process when isFinalizerWatchdogTimeout(thread, exception) is true, or instead call Sentry.captureMessage with WARNING level and attach a static fingerprint/tag like "finalizer-watchdog-timeout" so events are grouped; update the code around the isFinalizerWatchdogTimeout check (where logger.warn and Sentry.captureException are invoked) and mirror the non-fatal handling used for isNonFatalGcCleanupFailure to avoid repeated identical reports.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/fragments/resources/ColorFragment.java`:
- Around line 84-91: The loadColorsFromXML method currently catches IOException
which swallows FileNotFoundException declared on the signature and prevents the
caller's error snackbar; update loadColorsFromXML in ColorFragment so
FileNotFoundException is allowed to propagate: either add a specific catch for
FileNotFoundException that rethrows it (or remove FileNotFoundException from
being caught by catching IOException after rethrow), and keep a separate catch
for other IOExceptions to log/handle them; locate the InputStream creation and
the try-with-resources in loadColorsFromXML and ensure FileNotFoundException is
not consumed there.
In
`@layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/fragments/resources/StringFragment.java`:
- Around line 78-85: The FileNotFoundException thrown by new FileInputStream is
currently swallowed by the catch(IOException) block so the caller no longer sees
the error; modify loadStringsFromXML to preserve caller-visible
FileNotFoundException by adding a specific catch(FileNotFoundException e) that
rethrows e (so the method still declares throws FileNotFoundException), and keep
a separate catch(IOException e) to handle and log other IO failures (e.g.,
close-time errors) while leaving stringParser and stringList assignment as is;
reference loadStringsFromXML, ValuesResourceParser, stringParser, and stringList
when making the change.
---
Outside diff comments:
In
`@vectormaster/src/main/java/org/appdevforall/codeonthego/vectormaster/VectorMasterDrawable.java`:
- Around line 117-135: The parser-setup path can return early and leak the
FileInputStream (vectorStream); ensure vectorStream is closed on all
early-return paths by either wrapping the InputStream in a try-with-resources or
creating a single outer try/finally that closes vectorStream, and then move
XmlPullParserFactory/newPullParser setup and the main parse loop (xpp usage)
inside that scope; specifically update the code handling vectorStream,
XmlPullParserFactory, xpp, and the early returns that set vectorModel to null so
that vectorStream.close() always runs (or is auto-closed) before returning,
while leaving resources.getXml(resID) behavior unchanged.
In
`@vectormaster/src/main/java/org/appdevforall/codeonthego/vectormaster/VectorMasterView.java`:
- Around line 99-117: In VectorMasterView (method where vectorStream is used)
the XmlPullParserException path can return before the outer finally that closes
vectorStream, leaking the FileInputStream; update the catch block around
XmlPullParserFactory/newPullParser()/xpp.setInput(...) to explicitly close
vectorStream (or refactor so a single outer try/finally encloses both parser
setup and parsing) before setting vectorModel = null and returning, ensuring
vectorStream is always closed even on parser-setup failures.
---
Nitpick comments:
In `@app/src/main/java/com/itsaky/androidide/app/IDEApplication.kt`:
- Around line 214-218: The FinalizerWatchdogDaemon branch currently calls
Sentry.captureException for every occurrence (inside
isFinalizerWatchdogTimeout), which can flood Sentry; change this to a
rate-limited or grouped report: for example, add an AtomicBoolean (e.g.,
finalizerWatchdogReported) and only call Sentry.captureException once per
process when isFinalizerWatchdogTimeout(thread, exception) is true, or instead
call Sentry.captureMessage with WARNING level and attach a static
fingerprint/tag like "finalizer-watchdog-timeout" so events are grouped; update
the code around the isFinalizerWatchdogTimeout check (where logger.warn and
Sentry.captureException are invoked) and mirror the non-fatal handling used for
isNonFatalGcCleanupFailure to avoid repeated identical reports.
🪄 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: f3b5d5d2-b7b4-4f59-9c7e-aac6409615d0
📒 Files selected for processing (8)
app/src/main/java/com/itsaky/androidide/app/IDEApplication.ktcomposite-builds/build-deps/java-compiler/src/main/java/com/itsaky/androidide/zipfs2/ZipFileSystem.javaidetooltips/src/main/java/com/itsaky/androidide/idetooltips/ToolTipManager.ktlayouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/fragments/resources/ColorFragment.javalayouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/fragments/resources/StringFragment.javalayouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/managers/ValuesManager.javavectormaster/src/main/java/org/appdevforall/codeonthego/vectormaster/VectorMasterDrawable.javavectormaster/src/main/java/org/appdevforall/codeonthego/vectormaster/VectorMasterView.java
…to comply with declaration and snackbar usage
Daniel-ADFA
approved these changes
Apr 24, 2026
Contributor
Daniel-ADFA
left a comment
Daniel-ADFA
left a comment
There was a problem hiding this comment.
LGTM with a tiny comment :)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Regarding APPDEVFORALL-E8 in Sentry...
Bonus!