ADFA-3160 | Fix IllegalStateException by forcing main thread updates on ListView adapter#1462
ADFA-3160 | Fix IllegalStateException by forcing main thread updates on ListView adapter#1462jatezzz wants to merge 3 commits into
Conversation
…ifications Fix crash caused by modifying or notifying ListView adapter changes from background threads.
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 Walkthrough
Walkthrough
ChangesThread-safe session list refresh
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OpenGrep (1.23.0)termux/termux-app/src/main/java/com/termux/app/TermuxActivity.java┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.11][ERROR]: unable to find a config; path termux/termux-app/src/main/java/com/termux/app/TermuxService.java┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.11][ERROR]: unable to find a config; path 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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@termux/termux-app/src/main/java/com/termux/app/TermuxActivity.java`:
- Around line 976-982: Serialize the session-list update with the UI refresh by
ensuring the mutation in addNewSession() and the refresh in
termuxSessionListNotifyUpdated() happen on the main thread together, or by
having TermuxSessionListViewController work from an immutable snapshot instead
of the live mTermuxSessions ArrayList. Update the code paths around
addNewSession(), mTermuxSessions, and termuxSessionListNotifyUpdated() so
ListView never observes the list while a background executor is modifying it.
🪄 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: 55a7dfa3-6e3e-4712-8b8a-e218b0b46635
📒 Files selected for processing (1)
termux/termux-app/src/main/java/com/termux/app/TermuxActivity.java
…thread-only snapshot instead of sharing TermuxService's live session list mutated off the UI thread
|
@claude review once |
There was a problem hiding this comment.
Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit at claude.ai/admin-settings/claude-code.
Once credits are available, comment @claude review on this pull request to trigger a review.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@termux/termux-app/src/main/java/com/termux/app/TermuxActivity.java`:
- Line 631: `onServiceConnected()` still uses the live session list for the
working-directory lookup, which can race with concurrent session updates. Update
the lookup in `TermuxActivity.onServiceConnected()` to iterate over
`mTermuxService.getTermuxSessionsListSnapshot()` instead of
`getTermuxSessions()`, matching the snapshot-based approach already used for
`TermuxSessionsListViewController` and avoiding concurrent modification issues.
In `@termux/termux-app/src/main/java/com/termux/app/TermuxService.java`:
- Around line 899-907: `getTermuxSessionsListSnapshot()` assumes
`mShellManager.mTermuxSessions` is always mutated under the service monitor, but
`onTermuxSessionExited()` still removes sessions without taking that same lock.
Update `onTermuxSessionExited()` in `TermuxService` to synchronize on the same
monitor used by `getTermuxSessionsListSnapshot()` before touching
`mShellManager.mTermuxSessions`, so all session add/remove paths share the same
lock and the snapshot guarantee stays consistent.
🪄 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: 9643ae6a-a8fc-46a1-a7fe-0c2d8bb548d3
📒 Files selected for processing (3)
termux/termux-app/src/main/java/com/termux/app/TermuxActivity.javatermux/termux-app/src/main/java/com/termux/app/TermuxService.javatermux/termux-app/src/main/java/com/termux/app/terminal/TermuxSessionsListViewController.java
…working-dir lookup to keep getTermuxSessionsListSnapshot consistent and avoid concurrent-modification races
Description
This PR resolves a recurrent crash (
java.lang.IllegalStateException: The content of the adapter has changed but ListView did not receive a notification) inTermuxActivity.The crash happens because underlying session data changes are being processed on a background thread while the UI thread attempts a layout pass on
ListViewbeforenotifyDataSetChanged()executes on the main thread. By adding explicit safety scaffolding withLooper.myLooper() == Looper.getMainLooper(), we guarantee that layout dataset notifications dispatch correctly and sync immediately with the UI thread scheduler.Details
Screen.Recording.2026-06-29.at.12.55.52.PM.mov
Ticket
ADFA-3160
Observation
The addition of the
Looperverification guarantees thread compliance, eliminating race conditions between internal data collection steps and the Android layout rendering passes.Note that this is a defensive fix, as the issue could not be reproduced locally.