ADFA-2657 | Fix editor threading and selection logic#980
ADFA-2657 | Fix editor threading and selection logic#980
Conversation
📝 Walkthrough
Impact
Files/areas touched (high-level)
Lines changed (summary from PR): roughly +45 / -28 (as reported in PR metadata). WalkthroughConvert synchronous file-open paths to coroutine-based async flows: Changes
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)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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)
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. Comment |
There was a problem hiding this comment.
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.
app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt
Outdated
Show resolved
Hide resolved
Ensure openFile operations respect lifecycle scopes and handle UI updates on the main thread.
1b3aa0b to
77c270c
Compare
There was a problem hiding this comment.
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.
app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt
Outdated
Show resolved
Hide resolved
…or Java callers Replace blocking openFile usage with lifecycleScope-based coroutine execution and add openFileAsync to avoid runBlocking and potential ANRs.
There was a problem hiding this comment.
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
onResultthrows, the exception will propagate unhandled inlifecycleScope, 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.
| activity.openFileAsync(file, null, openedEditor -> { | ||
| if (openedEditor != null && openedEditor.getEditor() != null) { | ||
| editInEditor(openedEditor.getEditor(), edit); | ||
| } | ||
| return kotlin.Unit.INSTANCE; | ||
| }); |
There was a problem hiding this comment.
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.
Description
This PR refactors the file opening logic in
EditorHandlerActivityto improve thread safety and prevent potential UI freezes. By shifting from blocking calls to a coroutine-basedopenFileInternalmethod, 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
runBlockingwithwithContext(Dispatchers.IO)for image validation.openFileInternalas a suspend function to handle the core logic.openFileto either launch a coroutine or block appropriately based on the calling context.lifecycleScope.After changes
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.