diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationRelayService.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationRelayService.kt index 36acb126e2..dec5bcecb1 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationRelayService.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationRelayService.kt @@ -132,7 +132,6 @@ class NotificationRelayService : Service() { private val scope = CoroutineScope(Dispatchers.IO + SupervisorJob()) private var relayServiceCollectorJob: Job? = null private var connectedRelayCount = 0 - private var foregroundStarted = false override fun onBind(intent: Intent?): IBinder? = null @@ -140,7 +139,7 @@ class NotificationRelayService : Service() { super.onCreate() Log.d(TAG, "Service created") createNotificationChannel() - initializeForeground() + ensureForeground() } override fun onStartCommand( @@ -149,9 +148,16 @@ class NotificationRelayService : Service() { startId: Int, ): Int { Log.d(TAG, "Starting service") - // Safety: also call startForeground from onStartCommand in case - // onCreate didn't complete before onStartCommand fired (ntfy #1520) - initializeForeground() + // Every startForegroundService() call re-arms Android's "must call + // startForeground() within the timeout" requirement — including the repeated + // calls MainActivity.onResume fires on each resume, even when the service is + // already running and already foregrounded. We therefore call startForeground() + // on EVERY start command, not just the first. Skipping it on later starts (the + // old `if (foregroundStarted) return` guard) left the re-armed requirement + // unsatisfied and triggered ForegroundServiceDidNotStartInTimeException, which + // crashes the whole app. startForeground() is idempotent — repeated calls just + // refresh the existing notification. + if (!ensureForeground()) return START_NOT_STICKY startRelayConnection() return START_STICKY } @@ -206,10 +212,20 @@ class NotificationRelayService : Service() { super.onDestroy() } - private fun initializeForeground() { - if (foregroundStarted) return + /** + * Promotes the service to the foreground. Called on creation AND on every + * [onStartCommand], because each [Context.startForegroundService] re-arms the + * "must call startForeground() within the timeout" requirement and the old code's + * early-return-when-already-started skipped it, crashing the app with + * ForegroundServiceDidNotStartInTimeException. Calling startForeground() repeatedly + * is safe — it just refreshes the existing notification. + * + * Returns true if the service is foregrounded, false if promotion failed (in which + * case the service has already been told to stop). + */ + private fun ensureForeground(): Boolean { try { - val notification = buildNotification(0) + val notification = buildNotification(connectedRelayCount) ServiceCompat.startForeground( this, NOTIFICATION_ID, @@ -220,16 +236,26 @@ class NotificationRelayService : Service() { 0 }, ) - foregroundStarted = true + return true } catch (e: Exception) { + // Any failure to promote to the foreground leaves the service in a + // "started but not foregrounded" zombie state. Once startForegroundService() + // has been called, Android expects startForeground() within the timeout; if it + // never lands it fires ForegroundServiceDidNotStartInTimeException and crashes + // the whole app. Tearing the service down clears the OS's fgRequired flag, + // which cancels that pending timeout, so we stopSelf() on EVERY failure path — + // not just ForegroundServiceStartNotAllowedException. Other causes include + // OEM-specific RemoteException/IllegalStateException and resource lookup + // failures while building the notification. if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S && e is ForegroundServiceStartNotAllowedException ) { Log.w(TAG, "Foreground service start not allowed, stopping self") - stopSelf() } else { - Log.e(TAG, "Failed to start foreground", e) + Log.e(TAG, "Failed to start foreground, stopping self", e) } + stopSelf() + return false } }