Fix android mapper NullPointerException issue
Attempts to fix https://github.com/sqldelight/sqldelight/issues/6049
More detailed analysis and background are included in the issue, along with a minimal reproducible example.
Bug
This issue is surprisingly easy to trigger on Android devices, especially when running reactive queries over larger tables or on slower hardware.
A very small reproducible example is here:
https://github.com/JsFlo/sqldelight-mapper-npe-repro
And the same three files integrated directly into SQLDelight’s sample Android app (for easier step-through debugging) are here:
https://github.com/sqldelight/sqldelight/compare/master...JsFlo:sqldelight:reproAndroidNpeIssue?expand=1
Repro gist:
Start a reactive query (e.g., asFlow().mapToList()) and delete rows while the cursor is mid-iteration. Android’s CursorWindow may shrink due to the deletes. When this happens, Android does not throw an error, instead all get*() calls return null, even for NON NULL columns.
If the mapper generated !! (for NOT NULL columns) then this crashes with:
CursorWindow: Failed to read row 1285, column 0 from a window with 1285 rows, 2 columns
java.lang.NullPointerException
at UserQueries.selectAll$lambda$0(UserQueries.kt:17)
...
When all mapped columns are nullable, SQLDelight generates mappers without !!, so the behavior is:
- same cpp log is emitted ("Failed to read row X...")
get*()returns null silently- The row is treated as valid
- Incorrect null values flow downstream
If enough stale rows are read, the cursor eventually walks past the real number of rows and AndroidCursor.next() is called -> which eventually calls SQLiteCursor.onMove where fillWindow is called and because we're past the real number of rows an exception like this is thrown:
Row too big to fit into CursorWindow requiredPos=92, totalRows=80
android.database.sqlite.SQLiteBlobTooBigException
This can happen regularly in apps, and in practice can be triggered by deleting even a single row in the right conditions.
PR
This PR introduces:
1. Explicit detection of stale CursorWindow rows
A new StaleWindowException is thrown when the cursor’s logical position is no longer backed by the active window.
2. Per-row handling in awaitAsList()
Instead of crashing (due to !!) or emitting incorrect nulls (all-nullable schema), only the affected row is skipped. All valid rows still map correctly.
Why this is needed
On Android, when a cursor is iterating and concurrent DELETEs occur:
- The
CursorWindowmay shrink - Android does not throw
getLong(),getString(), etc. return null silently- A log entry is printed by native code:
CursorWindow: Failed to read row X, column 0 from a window with Y rows
What this PR changes
- Adds
StaleWindowException AndroidCursorchecks thatpositionis within[startPosition, startPosition + numRows)awaitAsList()catchesStaleWindowExceptionper row and skips the stale row
This prevents both mapper crashes and silent data corruption.
Notes on testing and observed behavior
While experimenting with tests, I noticed an additional behavior that the window-bounds check also intercepts certain SQLiteBlobTooBigException cases and at first I thought it meant that my solution was too broad but after looking at the issue some more I believe it legitimately fixes another related issue to the stale window issue.
Specifically, when the cursor iterates far enough past the real dataset (due to silent stale-row reads), Android eventually throws:
SQLiteBlobTooBigException: Row too big to fit into CursorWindow
requiredPos=92, totalRows=80
In this scenario, requiredPos > totalRows, which indicates the cursor has walked beyond the actual rows, a downstream effect of stale-window iteration, not a genuine “row too large” case.
Open questions for maintainers
- Is the cursor-bounds check appropriate at this layer of the driver?
- Would you prefer a different strategy, e.g., abort the entire query instead of skipping rows? I was thinking about my case for this and with reactive flows I'm not sure that would work well especially given how often this happens.
I have not added tests for the row-skipping behavior yet because I would like to confirm whether this direction is acceptable. Both for the detection and the awaitAsList behavior changes.
- [ ]
CHANGELOG.md's "Unreleased" section has been updated, if applicable.
Actually I think this might be a viable solution, the test i added surprised me because it would throw "BlobTooBig Row too big to fit into CursorWindow requiredPos=92, totalRows=80 when I commented out my fix but I believe that's because with enough stale window read attempts( when mapper doesn't generate !!) you'll run into that, misleading, error. But the real bug is the stale window read attempts.
I'll add the test to the PR