fix: process death crash loop and OOM on large logs#546
fix: process death crash loop and OOM on large logs#546jeremybernstein wants to merge 3 commits intoutkarshdalal:masterfrom
Conversation
prevents Settings (or other screens) from staying composed under XServer, wasting memory and causing bad back-navigation on game exit
read only last 256KB of log files instead of loading entire file into memory. also defer readText until dialog is actually visible.
📝 WalkthroughWalkthroughNavigation back-stack handling improved in PluviaMain using popUpTo to clear intermediate screens. XServerScreen adds error handling with try/catch for container retrieval and early-exit logic. SettingsGroupDebug introduces a tail-reading helper to limit log file memory consumption to 256 KB. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
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 |
|
These change appear to improve/fix random crashing I was getting when (re-)starting the app via Intents (e.g. from ES-DE), or when re-opening my clamshell device after it had been shut (during which time Android apparently killed off the process). |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/app/gamenative/ui/PluviaMain.kt`:
- Around line 1003-1006: When checking currentRoute ==
PluviaScreen.XServer.route and attempting to pop back to Home, capture the
boolean result of navController.popBackStack(PluviaScreen.Home.route, inclusive
= false); if it returns false (Home not in back stack) then explicitly navigate
to Home (e.g., navController.navigate(PluviaScreen.Home.route) with appropriate
options) as a fallback so the UI does not remain on XServer. Update the block
around currentRoute/PluviaScreen.XServer.route to perform the pop, test the
result, and call navController.navigate to PluviaScreen.Home.route when
popBackStack fails.
In `@app/src/main/java/app/gamenative/ui/screen/settings/SettingsGroupDebug.kt`:
- Around line 275-295: The readTail function risks OOM because
InputStream.skip() can skip fewer bytes than requested; replace the stream-based
tail logic in readTail with a RandomAccessFile-based approach: open the file
with RandomAccessFile, call seek(max(0, file.length() - MAX_LOG_DISPLAY_BYTES)),
read the remaining bytes via a buffered reader, drop the partial first line as
currently done, and prepend the "... (sizeKB, showing last XKB) ..." header;
keep the small-file path (file.readText()) and the exception handling intact and
reference MAX_LOG_DISPLAY_BYTES and readTail when making the change.
| if (currentRoute == PluviaScreen.XServer.route) { | ||
| navController.popBackStack() | ||
| // pop to Home to avoid loops after process death | ||
| navController.popBackStack(PluviaScreen.Home.route, inclusive = false) | ||
| } |
There was a problem hiding this comment.
Handle missing Home destination when popping from XServer.
If Home isn’t in the back stack, popBackStack(Home) returns false and the UI can remain on XServer. Add a fallback navigate-to-Home when the pop fails.
🔧 Proposed fix (fallback when pop fails)
- if (currentRoute == PluviaScreen.XServer.route) {
- // pop to Home to avoid loops after process death
- navController.popBackStack(PluviaScreen.Home.route, inclusive = false)
- }
+ if (currentRoute == PluviaScreen.XServer.route) {
+ // pop to Home to avoid loops after process death
+ val popped = navController.popBackStack(
+ PluviaScreen.Home.route,
+ inclusive = false,
+ )
+ if (!popped) {
+ navController.navigate(PluviaScreen.Home.route) {
+ popUpTo(navController.graph.startDestinationId) { inclusive = false }
+ }
+ }
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/app/gamenative/ui/PluviaMain.kt` around lines 1003 - 1006,
When checking currentRoute == PluviaScreen.XServer.route and attempting to pop
back to Home, capture the boolean result of
navController.popBackStack(PluviaScreen.Home.route, inclusive = false); if it
returns false (Home not in back stack) then explicitly navigate to Home (e.g.,
navController.navigate(PluviaScreen.Home.route) with appropriate options) as a
fallback so the UI does not remain on XServer. Update the block around
currentRoute/PluviaScreen.XServer.route to perform the pop, test the result, and
call navController.navigate to PluviaScreen.Home.route when popBackStack fails.
| private const val MAX_LOG_DISPLAY_BYTES = 256 * 1024L // 256 KB | ||
|
|
||
| // read the tail of a file, capped to avoid OOM on large logs | ||
| private fun readTail(file: File?): String? { | ||
| if (file == null || !file.exists()) return null | ||
| return try { | ||
| if (file.length() <= MAX_LOG_DISPLAY_BYTES) { | ||
| file.readText() | ||
| } else { | ||
| file.inputStream().use { stream -> | ||
| stream.skip(file.length() - MAX_LOG_DISPLAY_BYTES) | ||
| val text = stream.bufferedReader().readText() | ||
| // drop partial first line | ||
| val idx = text.indexOf('\n') | ||
| val trimmed = if (idx >= 0) text.substring(idx + 1) else text | ||
| "... (${file.length() / 1024}KB, showing last ${MAX_LOG_DISPLAY_BYTES / 1024}KB) ...\n$trimmed" | ||
| } | ||
| } | ||
| } catch (e: Exception) { | ||
| "Failed to read file: ${e.message}" | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Java InputStream.skip() guaranteed contract bytes skipped documentation
💡 Result:
java.io.InputStream.skip(long n) is not guaranteed to skip n bytes.
Javadoc contract (InputStream):
- It “skips over and discards
nbytes”, but may skip fewer bytes, possibly0, for many reasons (EOF is only one). - It returns the actual number of bytes skipped.
- If
nis negative,InputStream’s default contract is return0and skip nothing (subclasses may treat negative differently). (docs.oracle.com)
If you need a guarantee of skipping exactly n bytes:
- Use
InputStream.skipNBytes(long n)(Java 12+), which blocks untilnbytes are skipped, or throwsEOFExceptionif the stream ends first. (docs.oracle.com)
Citations:
- 1: https://docs.oracle.com/javase/8/docs/api/java/io/InputStream.html?utm_source=openai
- 2: https://docs.oracle.com/en/java/javase/12/docs/api/java.base/java/io/InputStream.html?utm_source=openai
🌐 Web query:
Kotlin safe tail file reading large files OOM RandomAccessFile seek
💡 Result:
To “tail” a large file safely in Kotlin (without OOM), don’t use readText(), readLines(), or Files.readAllLines()—they load the whole file. Instead, seek near the end and read fixed-size byte blocks.
Option A (recommended): Apache Commons IO ReversedLinesFileReader
It’s designed to read lines from the end using a bounded buffer (block size) and supports charset selection. (commons.apache.org)
import org.apache.commons.io.input.ReversedLinesFileReader
import java.nio.charset.StandardCharsets
import java.nio.file.Path
fun tailLines(path: Path, n: Int): List<String> =
ReversedLinesFileReader.builder()
.setPath(path)
.setCharset(StandardCharsets.UTF_8)
.setBufferSize(4096)
.get().use { r ->
buildList {
repeat(n) {
val line = r.readLine() ?: return@repeat
add(line)
}
}.asReversed()
}Option B (no extra deps): RandomAccessFile + seek() + backward scanning
RandomAccessFile lets you move the file pointer with seek() and read small buffers, so memory stays bounded. (docs.oracle.com)
High-level algorithm (robust for huge files):
seek(fileLength)(end)- Read backward in blocks (e.g., 8–64 KB), count
\n - Once you’ve found N newlines, compute the start position
seek(startPos)and read forward to EOF into an output sink (or collect lines)
This is the same general approach commonly recommended for “read last N lines” problems: scan backwards for line breaks, then jump to the desired position. (stackoverflow.com)
Option C: NIO FileChannel.position()
If you prefer NIO, FileChannel is seekable (position(long)), enabling the same bounded-buffer tail approach as Option B. (docs.oracle.com)
If you tell me whether you need last N lines or last N bytes, and the file encoding (UTF-8 vs unknown/mixed), I can provide a complete, production-safe Kotlin implementation for Option B (including edge cases like “no trailing newline” and Windows \r\n).
Citations:
- 1: https://commons.apache.org/proper/commons-io/apidocs/org/apache/commons/io/input/ReversedLinesFileReader.html?utm_source=openai
- 2: https://docs.oracle.com/javase/8/docs/api/java/io/RandomAccessFile.html?utm_source=openai
- 3: https://stackoverflow.com/questions/8494455/skip-x-last-lines-when-reading-a-text-file?utm_source=openai
- 4: https://docs.oracle.com/en/java/javase/19/docs/api/java.base/java/nio/channels/FileChannel.html?utm_source=openai
🏁 Script executed:
# Find the file and verify it exists
find . -type f -name "SettingsGroupDebug.kt" | head -5Repository: utkarshdalal/GameNative
Length of output: 142
🏁 Script executed:
# Check the file content around lines 275-295
cat -n ./app/src/main/java/app/gamenative/ui/screen/settings/SettingsGroupDebug.kt | sed -n '270,300p'Repository: utkarshdalal/GameNative
Length of output: 1199
🏁 Script executed:
# Check the context before and after, and the beginning of the file for imports
cat -n ./app/src/main/java/app/gamenative/ui/screen/settings/SettingsGroupDebug.kt | head -50Repository: utkarshdalal/GameNative
Length of output: 2599
readTail has OOM risk due to InputStream.skip() not being guaranteed.
InputStream.skip() may skip fewer bytes than requested, allowing readText() to still load most of a huge file into memory. Use RandomAccessFile.seek() for deterministic tail reading.
🔧 Proposed fix
} else {
- file.inputStream().use { stream ->
- stream.skip(file.length() - MAX_LOG_DISPLAY_BYTES)
- val text = stream.bufferedReader().readText()
- // drop partial first line
- val idx = text.indexOf('\n')
- val trimmed = if (idx >= 0) text.substring(idx + 1) else text
- "... (${file.length() / 1024}KB, showing last ${MAX_LOG_DISPLAY_BYTES / 1024}KB) ...\n$trimmed"
- }
+ val length = file.length()
+ val start = (length - MAX_LOG_DISPLAY_BYTES).coerceAtLeast(0)
+ java.io.RandomAccessFile(file, "r").use { raf ->
+ raf.seek(start)
+ val bytes = ByteArray((length - start).toInt())
+ raf.readFully(bytes)
+ val text = bytes.toString(Charsets.UTF_8)
+ // drop partial first line
+ val idx = text.indexOf('\n')
+ val trimmed = if (idx >= 0) text.substring(idx + 1) else text
+ "... (${length / 1024}KB, showing last ${MAX_LOG_DISPLAY_BYTES / 1024}KB) ...\n$trimmed"
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/app/gamenative/ui/screen/settings/SettingsGroupDebug.kt`
around lines 275 - 295, The readTail function risks OOM because
InputStream.skip() can skip fewer bytes than requested; replace the stream-based
tail logic in readTail with a RandomAccessFile-based approach: open the file
with RandomAccessFile, call seek(max(0, file.length() - MAX_LOG_DISPLAY_BYTES)),
read the remaining bytes via a buffered reader, drop the partial first line as
currently done, and prepend the "... (sizeKB, showing last XKB) ..." header;
keep the small-file path (file.readText()) and the exception handling intact and
reference MAX_LOG_DISPLAY_BYTES and readTail when making the change.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
1 participant
Summary
Test plan
Summary by CodeRabbit