Skip to content

ADFA-2657 | Fix editor threading and selection logic#980

Open
jatezzz wants to merge 2 commits intostagefrom
fix/ADFA-2657-editor-threading-and-selection
Open

ADFA-2657 | Fix editor threading and selection logic#980
jatezzz wants to merge 2 commits intostagefrom
fix/ADFA-2657-editor-threading-and-selection

Conversation

@jatezzz
Copy link
Collaborator

@jatezzz jatezzz commented Feb 13, 2026

Description

This PR refactors the file opening logic in EditorHandlerActivity to improve thread safety and prevent potential UI freezes. By shifting from blocking calls to a coroutine-based openFileInternal method, we ensure that file type checks (I/O operations) don't block the main thread while guaranteeing that UI updates (tab selection and editor focus) occur on the correct thread.

Details

  • Replaced runBlocking with withContext(Dispatchers.IO) for image validation.
  • Introduced openFileInternal as a suspend function to handle the core logic.
  • Implemented thread checks in openFile to either launch a coroutine or block appropriately based on the calling context.
  • Improved editor selection logic by ensuring it runs within the lifecycleScope.

After changes

Stressed thread simulation

Screen.Recording.2026-02-13.at.4.20.39.PM.mov

Ticket

ADFA-2657

Also fixes;

ADFA-2653
ADFA-2694
ADFA-2722
ADFA-2764
ADFA-2848

Observation

The use of Looper.myLooper() allows the activity to maintain backward compatibility with synchronous calls while moving toward a more reactive, non-blocking architecture.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 13, 2026

📝 Walkthrough
  • Converted editor file-opening flow to coroutines

    • openFile(...) became suspend (IEditorHandler signature updated).
    • Added openFileInternal suspend to hold core logic and moved image validation to withContext(Dispatchers.IO).
    • Introduced openFileAsync(file, selection? = null, onResult) helper for Java/non-coroutine callers.
    • Call sites updated to use lifecycleScope.launch (e.g., FileTreeActionHandler) and UI updates (tab selection, focus) are performed on the main thread via lifecycleScope.
  • Non-blocking behavior with compatibility handling

    • Most I/O and image detection now run off the main thread; UI-affecting steps run on Main.
    • openFile contains thread-detection logic (Looper.myLooper() checks) and will either launch coroutines or preserve a blocking path for synchronous callers.
    • IDL/Language client code (IDELanguageClientImpl) now uses openFileAsync and applies edits in the callback to avoid blocking the caller thread.
  • Other changes

    • Removed many runBlocking usages from call sites and reorganized error/early-return handling to fit coroutine boundaries.
    • Updated multiple callers to handle the async result path and moved selection/focus handling into coroutine flows.

⚠️ Risks / Best-practice violations

  • Breaking API/compatibility: changing openFile to suspend alters the IEditorHandler public API. Any callers compiled against the old synchronous signature (especially Java code) must use openFileAsync or wrappers; failing to update callers will cause compile-time or behavioral issues.
  • Inconsistent runtime semantics: thread-detection + dual code paths mean behavior differs depending on caller thread. Main-thread callers follow a non-blocking/callback path and may receive null or delayed editor results; background-thread callers may get blocking results. This can lead to subtle bugs where some callers expect immediate, non-null CodeEditorView.
  • Looper-based detection is brittle: using Looper.myLooper() == Looper.getMainLooper() can fail in tests, custom loopers, or advanced threading scenarios and may hide edge cases.
  • Potential for race conditions and lifecycle issues: callbacks (openFileAsync) and lifecycleScope launches must handle cancellation and activity destruction; applying edits or updating UI in callbacks risks interacting with destroyed activities if not guarded by lifecycle checks.
  • Partial removal of blocking: some code paths preserved blocking behavior for compatibility; retaining any blocking path undermines the goal of eliminating ANRs and may still risk thread starvation if misused.
  • Error/nullable propagation: callers that assume non-null editor instances may now observe nulls more often; ensure callers correctly handle null/failed open results.

Impact

  • Addresses: ADFA-2657 (primary). Also claims fixes for ADFA-2653, ADFA-2694, ADFA-2722, ADFA-2764, ADFA-2848.

Files/areas touched (high-level)

  • EditorHandlerActivity.kt — core refactor: suspend/openFileInternal/openFileAsync, IO/Main dispatcher boundaries, lifecycleScope usage.
  • IEditorHandler.kt — interface method signatures changed to suspend.
  • FileTreeActionHandler.kt — file-click handler updated to launch coroutine instead of calling openFile synchronously.
  • IDELanguageClientImpl.java — switched to openFileAsync and apply edits in callback to avoid blocking.

Lines changed (summary from PR): roughly +45 / -28 (as reported in PR metadata).

Walkthrough

Convert synchronous file-open paths to coroutine-based async flows: openFile becomes suspend, call sites launch coroutines or use openFileAsync, image detection and file I/O run on Dispatchers.IO, UI actions on Dispatchers.Main, and non-blocking callbacks used for external callers.

Changes

Cohort / File(s) Summary
Editor open flow
app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt
Made openFile a suspend function; moved image detection and file-indexing to IO context and UI interactions to Main; added openFileAsync helper; updated doOpenFile/openFileAndSelect to launch coroutines and handle post-init UI in coroutine flow.
Interface contract
app/src/main/java/com/itsaky/androidide/interfaces/IEditorHandler.kt
Changed openFile overloads to suspend in the IEditorHandler API signatures.
Tree click handler
app/src/main/java/com/itsaky/androidide/handlers/FileTreeActionHandler.kt
Replaced direct openFile call with lifecycleScope.launch { context.openFile(...) } to invoke suspend API from UI event.
LSP client edits
app/src/main/java/com/itsaky/androidide/lsp/IDELanguageClientImpl.java
Switched synchronous open-and-edit path to openFileAsync with a callback; apply edits when editor becomes available instead of blocking.

Sequence Diagram(s)

sequenceDiagram
    actor Caller
    participant FileTree as FileTreeActionHandler
    participant Activity as EditorHandlerActivity
    participant Coroutine as openFile (suspend)
    participant IO as Dispatchers.IO
    participant Main as Dispatchers.Main
    participant Editor as CodeEditorView

    Caller->>FileTree: file click
    FileTree->>Activity: lifecycleScope.launch { openFile(file) }
    Activity->>Coroutine: invoke suspend openFile(file)
    Coroutine->>IO: run image detection / file index
    alt is image
        IO-->>Coroutine: true
        Coroutine->>Main: withContext(Main) -> open image viewer
        Main-->>Coroutine: return null
    else not image
        IO-->>Coroutine: false
        Coroutine->>Main: withContext(Main) -> open/select tab, obtain editor
        Main->>Editor: create or retrieve CodeEditorView
        Editor-->>Main: CodeEditorView
        Main-->>Coroutine: CodeEditorView
    end
    Coroutine-->>Activity: CodeEditorView?
    Activity-->>Caller: result (via coroutine or openFileAsync callback)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • jomen-adfa
  • dara-abijo-adfa

Poem

🐰 I hopped through code with nimble feet,

Swapped blocking runs for coroutine fleet,
IO burrowed down, UI kept light,
Editors open quick — what a delight! 🥕

🚥 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 directly addresses the main objective of the PR: converting file-opening logic to a coroutine-based approach for improved thread safety and preventing UI freezes.
Description check ✅ Passed The description clearly explains the refactoring from blocking calls to coroutines, details the specific changes (runBlocking replacement, suspend function introduction, thread checks), and references related tickets.

✏️ 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 fix/ADFA-2657-editor-threading-and-selection

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.


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

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In
`@app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt`:
- Around line 495-499: getTabPositionForFileIndex can return -1 so add an
explicit guard after computing tabPosition in EditorHandlerActivity: if
tabPosition < 0 then log or return early instead of silently continuing; locate
the block that calls getTabPositionForFileIndex,
content.tabs.getTabAt(tabPosition) and tab.select and add the same style guard
used in onDocumentChange to either return or log a warning when tabPosition is
negative, ensuring you don't attempt to select a non-existent tab.
- Around line 510-521: openFile currently returns null when invoked on the main
thread; change it to always return the result of openFileInternal so callers
(e.g., IDELanguageClientImpl.performCodeAction) never get null. Replace the
launch/null branch with a synchronous invocation that runs the suspend
openFileInternal and returns its result; specifically, when isMainThread use
runBlocking(Dispatchers.Main.immediate) { openFileInternal(file, selection) }
(so the suspend executes on the main thread without losing the return value) and
when not main keep runBlocking { openFileInternal(file, selection) } — update
the openFile function to return the CodeEditorView? from openFileInternal
instead of returning null.

Ensure openFile operations respect lifecycle scopes and handle UI updates on the main thread.
@jatezzz jatezzz force-pushed the fix/ADFA-2657-editor-threading-and-selection branch from 1b3aa0b to 77c270c Compare February 18, 2026 13:43
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt`:
- Around line 510-521: onReadOpenedFilesCache currently calls openFile from
inside a withContext(Dispatchers.Main) block which causes openFile to launch
independent lifecycleScope coroutines that are not awaited, producing
non-deterministic tab selection; fix this by calling the suspend function
openFileInternal(file, selection) directly from onReadOpenedFilesCache (or
otherwise awaiting each openFileInternal call) so each file is opened
sequentially and the final tab.select() for cache.selectedFile runs last; update
references to openFile/openFileInternal in onReadOpenedFilesCache to call/await
openFileInternal rather than invoking openFile which launches unawaited
coroutines.

---

Duplicate comments:
In
`@app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt`:
- Around line 495-499: getTabPositionForFileIndex can return -1, so calling
content.tabs.getTabAt(tabPosition) may silently no-op; add an explicit guard
checking if tabPosition < 0 (or otherwise invalid) before calling getTabAt to
avoid silent failures. Locate the block using getTabPositionForFileIndex and
content.tabs.getTabAt, and change the logic to return/skip or handle the missing
tab when tabPosition < 0, only calling getTabAt and selecting the tab when
tabPosition >= 0 and the retrieved tab is non-null and not already selected.
- Around line 510-521: The main-thread branch currently fires a coroutine and
returns null, violating the CodeEditorView? contract; change the main-thread
path to synchronously obtain and return the result of openFileInternal instead
of lifecycleScope.launch. Specifically, replace lifecycleScope.launch {
openFileInternal(file, selection) } with a blocking call that returns
openFileInternal(file, selection) (e.g., runBlocking { openFileInternal(file,
selection) }) so both branches return the CodeEditorView? from openFileInternal;
ensure references to openFile, openFileInternal, lifecycleScope.launch and
runBlocking are updated accordingly.

…or Java callers

Replace blocking openFile usage with lifecycleScope-based coroutine execution and add openFileAsync to avoid runBlocking and potential ANRs.
@jatezzz jatezzz requested a review from itsaky-adfa February 18, 2026 14:51
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt (1)

502-510: Consider adding error handling around the callback invocation.

If onResult throws, the exception will propagate unhandled in lifecycleScope, potentially crashing the app. A try-catch around the callback would make this more robust, especially since it's a public API consumed by Java callers.

♻️ Proposed defensive wrapping
 fun openFileAsync(
 	file: File,
 	selection: Range? = null,
 	onResult: (CodeEditorView?) -> Unit
 ) {
 	lifecycleScope.launch {
-		onResult(openFile(file, selection))
+		val result = openFile(file, selection)
+		try {
+			onResult(result)
+		} catch (e: Exception) {
+			log.error("Error in openFileAsync callback", e)
+		}
 	}
 }
🤖 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/activities/editor/EditorHandlerActivity.kt`
around lines 502 - 510, Wrap the callback invocation in openFileAsync inside a
try-catch so exceptions thrown by the onResult callback (from Java or Kotlin
callers) don’t escape the lifecycleScope coroutine; call openFile(file,
selection) as before, then invoke onResult(...) inside a try { ... } catch (t:
Throwable) { /* log via logger or processLogger and swallow/handle gracefully */
} to prevent crashes and record the error; reference openFileAsync, openFile,
lifecycleScope, and onResult when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/src/main/java/com/itsaky/androidide/lsp/IDELanguageClientImpl.java`:
- Around line 255-260: The async file-open callback in applyActionEdits uses
activity.openFileAsync which fires a coroutine and returns before editInEditor
runs, so applyActionEdits can return Boolean.TRUE before edits are applied;
update applyActionEdits to track and await pending async edits (e.g., create a
CompletableDeferred or CountDownLatch per call and complete/count down inside
the openFileAsync callback after editInEditor finishes) and suspend/wait for
that completion before returning; update callers (or
TaskExecutor.executeAsyncProvideError) to respect the awaited result so the
progress dialog and success reporting only occur after the deferred/countdown
completes.

---

Nitpick comments:
In
`@app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt`:
- Around line 502-510: Wrap the callback invocation in openFileAsync inside a
try-catch so exceptions thrown by the onResult callback (from Java or Kotlin
callers) don’t escape the lifecycleScope coroutine; call openFile(file,
selection) as before, then invoke onResult(...) inside a try { ... } catch (t:
Throwable) { /* log via logger or processLogger and swallow/handle gracefully */
} to prevent crashes and record the error; reference openFileAsync, openFile,
lifecycleScope, and onResult when making the change.

Comment on lines +255 to +260
activity.openFileAsync(file, null, openedEditor -> {
if (openedEditor != null && openedEditor.getEditor() != null) {
editInEditor(openedEditor.getEditor(), edit);
}
return kotlin.Unit.INSTANCE;
});
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Async edit for unopened files may complete after applyActionEdits returns Boolean.TRUE.

openFileAsync launches a fire-and-forget coroutine, so the method returns Boolean.TRUE (line 266) before the edit callback executes. In the TaskExecutor.executeAsyncProvideError path (lines 210–220), this means the progress dialog is dismissed and "success" is reported before these edits are actually applied.

This is likely an acceptable trade-off to avoid the previous runBlocking deadlock risk, but callers that rely on all edits being committed when applyActionEdits returns should be aware of this behavioral change. If strict ordering matters, consider tracking pending async edits with a CompletableDeferred or a countdown mechanism.

🤖 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/lsp/IDELanguageClientImpl.java`
around lines 255 - 260, The async file-open callback in applyActionEdits uses
activity.openFileAsync which fires a coroutine and returns before editInEditor
runs, so applyActionEdits can return Boolean.TRUE before edits are applied;
update applyActionEdits to track and await pending async edits (e.g., create a
CompletableDeferred or CountDownLatch per call and complete/count down inside
the openFileAsync callback after editInEditor finishes) and suspend/wait for
that completion before returning; update callers (or
TaskExecutor.executeAsyncProvideError) to respect the awaited result so the
progress dialog and success reporting only occur after the deferred/countdown
completes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments