From 17f8aad9bff3d40a352446c72c32928addcbecf0 Mon Sep 17 00:00:00 2001 From: Mike Miller Date: Tue, 26 May 2026 14:48:59 -0400 Subject: [PATCH 1/2] Fix InboxListView NPE when switching tabs openVisibleMessages was launched on Dispatchers.IO and called LinearLayoutManager.findFirst/LastCompletelyVisibleItemPosition() from a worker thread. Those APIs walk RecyclerView children and read View.getLayoutParams(), which is not thread-safe and races with the layout pass. Switching tabs while the list was settling reliably crashed with a NullPointerException inside ViewBoundsCheck.findOneViewWithinBoundFlags. Resolve the visible index range and snapshot the message list on the main thread, then move only the markAsOpened() network calls to Dispatchers.IO via withContext. Bound the snapshot subList defensively against stale indexes. Co-authored-by: Cursor --- .../courier/android/ui/inbox/InboxListView.kt | 63 +++++++++---------- 1 file changed, 30 insertions(+), 33 deletions(-) diff --git a/android/src/main/java/com/courier/android/ui/inbox/InboxListView.kt b/android/src/main/java/com/courier/android/ui/inbox/InboxListView.kt index 069e821..4288a22 100644 --- a/android/src/main/java/com/courier/android/ui/inbox/InboxListView.kt +++ b/android/src/main/java/com/courier/android/ui/inbox/InboxListView.kt @@ -33,6 +33,7 @@ import kotlinx.coroutines.async import kotlinx.coroutines.awaitAll import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext internal class InboxListView @JvmOverloads constructor( context: Context, @@ -382,52 +383,48 @@ internal class InboxListView @JvmOverloads constructor( } // Opens all the current messages - // Performs this on an async thread - // Fails silently - private fun openVisibleMessages() = coroutineScope.launch(Dispatchers.IO) { + // Resolves visible items on the main thread (RecyclerView/LayoutManager state is + // not thread-safe and reading it from a worker thread races with the layout + // pass — most easily reproduced when switching tabs while the list is settling, + // which produced a NullPointerException inside ViewBoundsCheck). + // Network calls are then dispatched to IO. Fails silently. + private fun openVisibleMessages() = coroutineScope.launch(Dispatchers.Main) { // Ensure we have a user and messages to look through if (!Courier.shared.isUserSignedIn || mutatedMessageId != null || messagesAdapter.messages.isEmpty()) { return@launch } - // Get all the visible items - layoutManager?.let { manager -> + // Resolve visible indexes + snapshot messages on the main thread + val manager = layoutManager ?: return@launch + val firstIndex = manager.findFirstCompletelyVisibleItemPosition() + val lastIndex = manager.findLastCompletelyVisibleItemPosition() - // Get the indexes - val firstIndex = manager.findFirstCompletelyVisibleItemPosition() - val lastIndex = manager.findLastCompletelyVisibleItemPosition() - - // Avoid index out of bounds - if (firstIndex == -1 || lastIndex == -1) { - return@let - } - - try { - - // Find the messages - val messagesToOpen = messagesAdapter.messages.subList(firstIndex, lastIndex).filter { !it.isOpened }.map { message -> - - if (message.isOpened) { - return@map null - } - - // Mark the message as opened - return@map async { - message.markAsOpened() - } + if (firstIndex == -1 || lastIndex == -1) { + return@launch + } - } + val snapshot = messagesAdapter.messages.toList() + val safeFirst = firstIndex.coerceAtLeast(0) + val safeLast = lastIndex.coerceAtMost(snapshot.size) + if (safeFirst >= safeLast) { + return@launch + } - // Perform all the changes together - messagesToOpen.filterNotNull().awaitAll() + val messagesToOpen = snapshot.subList(safeFirst, safeLast).filter { !it.isOpened } + if (messagesToOpen.isEmpty()) { + return@launch + } + // Run network work off the main thread + withContext(Dispatchers.IO) { + try { + messagesToOpen.map { message -> + async { message.markAsOpened() } + }.awaitAll() } catch (e: Exception) { - Courier.shared.client?.error(e.message) - } - } } From 54256b195e65805faf237136dbfa6f16d1b6e6d6 Mon Sep 17 00:00:00 2001 From: Mike Miller Date: Tue, 26 May 2026 14:54:40 -0400 Subject: [PATCH 2/2] Drop unneeded message-list snapshot in openVisibleMessages The .toList() snapshot was added defensively, but adapter mutations (setInbox / addPage / addMessage / removeMessage) all run on the main thread, and openVisibleMessages now does too. The defensive copy was adding O(totalMessages) main-thread work on every scroll event for no benefit. Iterable.filter already returns a fresh List, so the IO block can still iterate messagesToOpen safely if the underlying adapter list mutates after we hand it off. Co-authored-by: Cursor --- .../com/courier/android/ui/inbox/InboxListView.kt | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/android/src/main/java/com/courier/android/ui/inbox/InboxListView.kt b/android/src/main/java/com/courier/android/ui/inbox/InboxListView.kt index 4288a22..f63b0cb 100644 --- a/android/src/main/java/com/courier/android/ui/inbox/InboxListView.kt +++ b/android/src/main/java/com/courier/android/ui/inbox/InboxListView.kt @@ -395,23 +395,26 @@ internal class InboxListView @JvmOverloads constructor( return@launch } - // Resolve visible indexes + snapshot messages on the main thread + // Resolve visible items on the main thread. RecyclerView/LayoutManager + // state and the adapter list are both confined to the main thread, so + // these reads are safe and cheap (visible children are bounded). val manager = layoutManager ?: return@launch val firstIndex = manager.findFirstCompletelyVisibleItemPosition() val lastIndex = manager.findLastCompletelyVisibleItemPosition() - if (firstIndex == -1 || lastIndex == -1) { return@launch } - val snapshot = messagesAdapter.messages.toList() + val messages = messagesAdapter.messages val safeFirst = firstIndex.coerceAtLeast(0) - val safeLast = lastIndex.coerceAtMost(snapshot.size) + val safeLast = lastIndex.coerceAtMost(messages.size) if (safeFirst >= safeLast) { return@launch } - val messagesToOpen = snapshot.subList(safeFirst, safeLast).filter { !it.isOpened } + // .filter produces a new List, so it's safe to pass into the IO block + // even if the underlying adapter list mutates afterwards. + val messagesToOpen = messages.subList(safeFirst, safeLast).filter { !it.isOpened } if (messagesToOpen.isEmpty()) { return@launch }