ADFA-3130 | Fix memory usage chart crash#1470
Conversation
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.
eef2deb to
b7a06d4
Compare
|
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
WalkthroughAdds a ChangesSafe chart rendering
Estimated code review effort: 1 (Trivial) | ~5 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/java/com/itsaky/androidide/ui/SafeLineChart.kt (1)
54-65: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueNon-atomic counter racing with the exact hazard being guarded against.
skippedFrames++is a non-atomic read-modify-write. Per this class's own documentation,onDrawcan be invoked concurrently from more than one thread (main + Sentry Session Replay background thread) — the same condition that causes theIndexOutOfBoundsExceptionbeing caught here. Concurrent increments can lose updates or produce an inconsistent log cadence/count. Impact is limited to logging accuracy (not a crash), so this is low priority, but consider making the counter atomic for correctness.🔧 Optional fix using an atomic counter
- private var skippedFrames = 0L + private val skippedFrames = java.util.concurrent.atomic.AtomicLong(0) override fun onDraw(canvas: Canvas) { try { super.onDraw(canvas) } catch (e: IndexOutOfBoundsException) { // Transient race in MPAndroidChart's axis renderer (see class doc). Skip this frame. // Only log occasionally to avoid flooding logcat, since onDraw runs every frame. - if (skippedFrames++ % 60L == 0L) { - log.warn("Skipped {} chart frame(s) due to a transient axis-rendering race", skippedFrames, e) + val count = skippedFrames.incrementAndGet() + if (count % 60L == 1L) { + log.warn("Skipped {} chart frame(s) due to a transient axis-rendering race", count, e) } } }🤖 Prompt for 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. In `@app/src/main/java/com/itsaky/androidide/ui/SafeLineChart.kt` around lines 54 - 65, The skippedFrames counter in SafeLineChart.onDraw is being updated with a non-atomic read-modify-write while the method may run concurrently, so replace the plain Long counter with an atomic counter and use its atomic increment/read APIs when deciding whether to log. Keep the fix localized to the onDraw override and the skippedFrames field so the log cadence and count remain correct under concurrent draws.
🤖 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.
Nitpick comments:
In `@app/src/main/java/com/itsaky/androidide/ui/SafeLineChart.kt`:
- Around line 54-65: The skippedFrames counter in SafeLineChart.onDraw is being
updated with a non-atomic read-modify-write while the method may run
concurrently, so replace the plain Long counter with an atomic counter and use
its atomic increment/read APIs when deciding whether to log. Keep the fix
localized to the onDraw override and the skippedFrames field so the log cadence
and count remain correct under concurrent draws.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 659e7bd9-f6bf-4868-b503-acebb37eb279
📒 Files selected for processing (2)
app/src/main/java/com/itsaky/androidide/ui/SafeLineChart.ktapp/src/main/res/layout/layout_mem_usage.xml
Introduced SafeLineChart to safely catch IndexOutOfBoundsException caused by MPAndroidChart thread-safety issues.
b7a06d4 to
69fa4e4
Compare
Description
Replaced the standard MPAndroidChart
LineChartwith a customSafeLineChartinlayout_mem_usage.xml. This prevents the IDE from crashing due to anIndexOutOfBoundsExceptioncaused by an axis-rendering race condition in the MPAndroidChart library. This race condition is typically triggered when Sentry Session Replay records the screen drawing on a background thread concurrently with main-thread updates.Details
SafeLineChart.ktwhich extendsLineChart.onDraw()with a try-catch block to swallow the transientIndexOutOfBoundsExceptionand skip the corrupted frame.Screen.Recording.2026-07-02.at.9.41.53.AM.mov
Ticket
ADFA-3130
Observation
Because the memory chart is a non-critical diagnostic view, dropping an occasional frame is much safer than crashing the entire IDE. The view cleanly recovers on the next
invalidate()call.