From 67b2516571e49330accf3403a3a8d253cd03ad5e Mon Sep 17 00:00:00 2001
From: SiteRelEnby <125829806+SiteRelEnby@users.noreply.github.com>
Date: Thu, 27 Aug 2026 18:40:24 -0400
Subject: [PATCH] fix(widget): stop the widget refresh crashing on a null
 PendingResult

Two crashes reported from the field:

  NullPointerException: Attempt to invoke virtual method
  'void android.content.BroadcastReceiver$PendingResult.finish()'
  on a null object reference
    at WidgetRefreshDispatch.kt:47

goAsync() hands out the receiver's PendingResult exactly once per dispatch and
clears its own reference, so a second caller in the same onReceive gets null.
GlanceAppWidgetReceiver.onUpdate calls goAsync() for its own compose, and all
six receivers here call refreshWidgets after super.onUpdate(), so ours always
gets null. Treating it as non-null threw out of the finally block, and an
exception escaping a coroutine launched into a bare scope takes the process
with it.

Confirmed against the compiled Glance 1.1.0 class rather than inferred:
GlanceAppWidgetReceiver.onUpdate invokes CoroutineBroadcastReceiverKt.goAsync,
which calls BroadcastReceiver.goAsync() and finishes the result in its own
continuation. Glance wraps its finish() in a catch and logs; ours did not.

finish() is now null-safe, and the whole call is wrapped since it also throws
if the broadcast has already been finished.

This does not restore the keep-alive the original comment claimed, because
these callers never had it: Glance holds the only PendingResult. A refresh cut
short by process death leaves a stale widget until the next tick, which is what
already happens today. The comment now says so instead of describing a
protection that is not there, and names the durable fix (enqueue the refresh as
expedited work rather than running it in the receiver) for when someone picks
it up.
---
 .../sheaf/widget/WidgetRefreshDispatch.kt     | 30 +++++++++++++++----
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/sheaf/app/src/main/java/systems/lupine/sheaf/widget/WidgetRefreshDispatch.kt b/sheaf/app/src/main/java/systems/lupine/sheaf/widget/WidgetRefreshDispatch.kt
index eda7de2..9445e42 100644
--- a/sheaf/app/src/main/java/systems/lupine/sheaf/widget/WidgetRefreshDispatch.kt
+++ b/sheaf/app/src/main/java/systems/lupine/sheaf/widget/WidgetRefreshDispatch.kt
@@ -14,10 +14,17 @@ import kotlinx.coroutines.withTimeoutOrNull
 
 private const val TAG = "WidgetRefresh"
 
-// A widget refresh hits the network, so it outlives onUpdate. Without goAsync()
-// the receiver is finished the moment onUpdate returns and the process becomes
-// killable, so the refresh can be cut off mid-request and the widget just stays
-// stale. goAsync() keeps the process alive until finish() is called.
+// A widget refresh hits the network, so it outlives onUpdate. Once the receiver
+// finishes, the process becomes killable and the refresh can be cut off
+// mid-request, leaving the widget stale until the next tick.
+//
+// goAsync() is how a receiver asks to stay alive, but there is only one
+// PendingResult per dispatch and GlanceAppWidgetReceiver.onUpdate claims it for
+// its own compose before any of this runs. So in practice these refreshes are
+// NOT holding the broadcast open, and have not been since this was written.
+// Losing a refresh to process death is a stale widget, not lost data, so this
+// is a known limitation rather than a live bug; the durable fix is to enqueue
+// the refresh as expedited work instead of doing it in the receiver.
 //
 // The broadcast still has a hard deadline (10s in the foreground, 60s in the
 // background) before the system complains, so the work is bounded well inside
@@ -31,7 +38,13 @@ fun BroadcastReceiver.refreshWidgets(
     appWidgetIds: IntArray,
     action: ActionCallback,
 ) {
-    val pending = goAsync()
+    // goAsync() hands out the receiver's PendingResult exactly once and nulls
+    // its own reference, so a second caller in the same dispatch gets null.
+    // GlanceAppWidgetReceiver.onUpdate already calls it for its own compose,
+    // and every caller here runs after super.onUpdate(), so null is the normal
+    // case rather than an edge one. Treating it as non-null threw out of the
+    // finally below and took the process with it.
+    val pending: BroadcastReceiver.PendingResult? = goAsync()
     CoroutineScope(SupervisorJob() + Dispatchers.IO).launch {
         try {
             withTimeoutOrNull(REFRESH_TIMEOUT_MS) {
@@ -44,7 +57,12 @@ fun BroadcastReceiver.refreshWidgets(
         } catch (t: Throwable) {
             Log.w(TAG, "widget refresh failed: ${t::class.simpleName}")
         } finally {
-            pending.finish()
+            // Nothing to release when Glance holds the broadcast; the refresh
+            // still runs, it just isn't protected from the process being
+            // killed. A cut-short refresh leaves the widget stale until the
+            // next tick, which is what happens today anyway.
+            runCatching { pending?.finish() }
+                .onFailure { Log.w(TAG, "could not finish broadcast: ${it::class.simpleName}") }
         }
     }
 }