Skip to content

Commit e6ef957

Browse files
taayyohhbahdotsh
andauthored
fix(bindings): promote MeshForegroundService to foreground in onCreate + add Stop action (#278)
* fix(bindings): promote MeshForegroundService to foreground in onCreate Android gives an app 5 seconds after startForegroundService() to reach startForeground(). MeshForegroundService currently calls startForeground() from onStartCommand, which on cold start and on resume can be delayed past that window by JS-thread initialisation and main-thread work. When that happens the OS terminates the process with a fatal RemoteServiceException. Move the startForeground() call into onCreate so the deadline is unreachable regardless of what runs on the main thread after service creation. The subsequent startForeground() calls in onStartCommand are idempotent on the same service instance and are safe re-promotes. Also add a "Stop" notification action so the user can shut mesh down from the notification shade. It routes back through the service's own ACTION_STOP handler via PendingIntent.getForegroundService on Android O+ (getService on older releases), so no separate BroadcastReceiver is required and the delivery is legal under the background service-start restrictions that apply when the user taps the action while the app is in the background. Observed impact: this fix has been shipping in-app via a patch-package override since v67 of the MINE app; the underlying crash was the top ANR-adjacent fatal on Redmi Note 8 Pro (Android 11), fingerprinted by Sentry as REACT-NATIVE-5Z. Retires the MINE-side patch tracked by Linear OFF-1801. * fix(bindings): make the mesh notification Stop action stop the mesh The Stop action added alongside the onCreate promotion routes straight back into the service's own ACTION_STOP handler, which drops the keep-alive and nothing else. But this service is *only* a keep-alive — the module owns the protocol and the transports, exactly as the class comment has said all along. So tapping Stop cleared the notification and the foreground protection while BLE, WiFi Direct, Nostr and the process scheduler kept right on running, with nothing told to JS. The user sees "mesh off" while the radios keep draining the battery until the OS gets around to reaping the process. Give the notification its own action that hands off to a host callback instead. The module runs the same teardown as its JS-facing stop() and emits mesh_stopped_by_user so app state can't silently diverge, and the service stays up until that teardown comes back around through ACTION_STOP — clearing the notification while the mesh is still running is exactly the lie we're fixing here. With no host registered we still drop the keep-alive, because a dead button is its own kind of bug. While at it, the try/catch only wrapped the onCreate promotion; the other two call sites were bare. That matters, because a connectedDevice promotion throws once the Nearby-Devices permissions are revoked — so the guarded failure came straight back uncaught a few milliseconds later. All three share a helper now. It is not immunity: if the instance came from startForegroundService() and promotion genuinely fails, the system still raises its own timeout kill, and the helper says so. And stop() is a no-op when nothing is running, since creating an instance purely to tear it down now means a notification flash on a path the app already runs twice. The new event tag is emitted by the bridge, not by the core enum, so it joins the bridge-only allowlist in the types.ts drift guard. That guard is doing exactly what it was built for — it caught the omission before I did. The tests pin the part that is invisible in the source: the service is in the foreground by the end of onCreate, with no start command delivered. Move that promotion back into onStartCommand and the code still reads fine while the five-second deadline is quietly reachable again. * fix(bindings): serialize mesh teardown and always run its tail The notification Stop action added earlier in this branch runs the same teardown as the JS-facing stop(), except it does it on its own thread. Nothing was stopping the two from overlapping, and a user tapping Stop while the app foregrounds and calls stop() is not an exotic scenario. Two threads interleaving through stopTransportsAndProtocol double-stop every transport mid-pass. Worse, if both throw — and BLE teardown throwing on real devices is the entire reason #279 and #280 exist — neither pass ever reaches the remaining transports, the keep-alive service, or the protocol core. The user-stop path then clears the notification and tells JS the mesh is down while five transports are still burning battery. That is precisely the lie this branch was written to prevent. The JS path had the same hole without needing a race at all: one throwing transport skipped the foreground-service stop and protocol.stop() outright, so the notification kept advertising an active mesh over a half-dead stack. So serialize the shared teardown, and move the keep-alive and core shutdown into a finally. The second entrant through the lock re-runs the stops after a completed pass, which is their idempotent no-op path. The exception still propagates, so stop() rejects exactly as it did before. While at it, drop the now-redundant keep-alive stop from the user-stop path — the shared function guarantees it now. * test(bindings): stop leaking the start-request flag between tests The service tracks start intent in a companion field, and onDestroy is what clears it. A test that calls start() without ever creating an instance therefore leaves that flag set for whatever runs next — there is no service instance for the fixture to destroy. Nothing is red today, because the one test that cares re-establishes its own precondition. That is luck, not design, and it lasts exactly until someone adds a test between those two. So just call stop() in the fixture. It no-ops once both flags are already down, which costs nothing on every other test. --------- Co-authored-by: bahdotsh <appu.yess@gmail.com>
1 parent 137715b commit e6ef957

6 files changed

Lines changed: 536 additions & 40 deletions

File tree

bindings/react-native/android/build.gradle

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,12 @@ dependencies {
8686

8787
testImplementation 'junit:junit:4.13.2'
8888
testImplementation 'org.robolectric:robolectric:4.16'
89+
// NotificationCompat reaches the production classpath transitively through
90+
// react-android, which is compileOnly in the standalone/CI harness — so the
91+
// JVM tests have to bring it themselves. Test-only: consumers keep getting
92+
// it from the host app. Hold at 1.13.x, the last line that builds against
93+
// compileSdk 34.
94+
testImplementation 'androidx.core:core:1.13.1'
8995
// Real org.json for JVM unit tests (the android.jar org.json is a stub
9096
// that throws) — needed by RelayControlOpTranslatorTest.
9197
testImplementation 'org.json:json:20260719'

bindings/react-native/android/src/main/java/com/offlineprotocol/MeshForegroundService.kt

Lines changed: 152 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package com.offlineprotocol
33
import android.app.Notification
44
import android.app.NotificationChannel
55
import android.app.NotificationManager
6+
import android.app.PendingIntent
67
import android.app.Service
78
import android.content.Context
89
import android.content.Intent
@@ -24,7 +25,8 @@ import androidx.core.app.NotificationCompat
2425
* The service itself does NOT own the protocol or transport instances.
2526
* It exists solely to prevent the OS from killing the process while mesh
2627
* networking is active. Protocol and transport lifecycle remains in
27-
* OfflineProtocolModule.
28+
* OfflineProtocolModule — including the notification's Stop action, which
29+
* hands off to [onStopRequestedByUser] instead of tearing down here.
2830
*/
2931
class MeshForegroundService : Service() {
3032

@@ -37,10 +39,28 @@ class MeshForegroundService : Service() {
3739
private const val ACTION_START = "com.offlineprotocol.action.START_MESH"
3840
private const val ACTION_STOP = "com.offlineprotocol.action.STOP_MESH"
3941

42+
/**
43+
* Delivered by the notification's Stop action. Deliberately distinct
44+
* from [ACTION_STOP]: that one is the host telling the service that
45+
* mesh is *already* going down, while this one is the user asking for
46+
* a teardown the host has not started yet and must run itself.
47+
*/
48+
private const val ACTION_STOP_FROM_NOTIFICATION =
49+
"com.offlineprotocol.action.STOP_MESH_FROM_NOTIFICATION"
50+
4051
@Volatile
4152
var isRunning: Boolean = false
4253
private set
4354

55+
/**
56+
* Set by [start], cleared by [stop] and [onDestroy]. Tracks intent
57+
* rather than state: the service is created asynchronously, so a
58+
* `stop()` racing a just-issued `start()` sees [isRunning] still false
59+
* while a service is on its way up.
60+
*/
61+
@Volatile
62+
private var startRequested: Boolean = false
63+
4464
/**
4565
* Callback invoked when the service is restarted after a process kill
4666
* (START_STICKY re-delivery). The host module should set this so it can
@@ -49,7 +69,24 @@ class MeshForegroundService : Service() {
4969
@Volatile
5070
var onServiceRestarted: (() -> Unit)? = null
5171

72+
/**
73+
* Callback invoked when the user taps "Stop" on the service
74+
* notification. The host module must set this and run the same
75+
* teardown as its JS-facing `stop()`: this service is only a
76+
* keep-alive, so dropping it alone would leave BLE/WiFi-Direct/Nostr
77+
* and the process scheduler running with no foreground protection and
78+
* no JS-visible state change — the user sees "mesh off" while the
79+
* radios keep draining until the OS reaps the process.
80+
*
81+
* Held in a companion field, so a host that captures itself here must
82+
* null it out on teardown or it pins that host for the process
83+
* lifetime.
84+
*/
85+
@Volatile
86+
var onStopRequestedByUser: (() -> Unit)? = null
87+
5288
fun start(context: Context) {
89+
startRequested = true
5390
val intent = Intent(context, MeshForegroundService::class.java).apply {
5491
action = ACTION_START
5592
}
@@ -61,6 +98,18 @@ class MeshForegroundService : Service() {
6198
}
6299

63100
fun stop(context: Context) {
101+
// Skip when there is nothing to stop. Callers invoke this from
102+
// both stop() and invalidate(), so a stop-after-stop would
103+
// otherwise create an instance purely to tear it down — and since
104+
// onCreate now promotes to the foreground, that means a visible
105+
// notification flash during app teardown.
106+
//
107+
// Both flags are needed: startRequested covers a stop racing a
108+
// service that is still coming up (isRunning not yet set), and
109+
// isRunning covers an instance we never requested — a START_STICKY
110+
// restart after process death, which resets the statics.
111+
if (!startRequested && !isRunning) return
112+
startRequested = false
64113
val intent = Intent(context, MeshForegroundService::class.java).apply {
65114
action = ACTION_STOP
66115
}
@@ -79,28 +128,39 @@ class MeshForegroundService : Service() {
79128
override fun onCreate() {
80129
super.onCreate()
81130
createNotificationChannel()
131+
// Enter the foreground here rather than waiting for onStartCommand.
132+
// Android gives an app 5 seconds after startForegroundService() to
133+
// call startForeground(); if onStartCommand is delayed past that
134+
// window (JS-thread initialization on cold start, main-thread work
135+
// during app resume on mid-range devices), the OS terminates the
136+
// process with a fatal RemoteServiceException. Promoting in
137+
// onCreate — which runs before any onStartCommand dispatch — makes
138+
// the deadline unreachable. Subsequent startForeground() calls in
139+
// onStartCommand are idempotent on the same service instance and
140+
// remain safe re-promotes.
141+
promoteToForeground()
82142
}
83143

84144
override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int {
85145
when (intent?.action) {
86146
ACTION_STOP -> {
87147
Log.i(TAG, "Stopping mesh foreground service")
88-
isRunning = false
89-
stopForeground(STOP_FOREGROUND_REMOVE)
90-
stopSelf()
148+
stopForegroundAndSelf()
91149
return START_NOT_STICKY
92150
}
151+
ACTION_STOP_FROM_NOTIFICATION -> {
152+
Log.i(TAG, "User requested mesh stop from the notification")
153+
return handleUserStopRequest()
154+
}
93155
ACTION_START -> {
94156
Log.i(TAG, "Starting mesh foreground service")
95-
startForeground(NOTIFICATION_ID, buildNotification())
96-
isRunning = true
157+
promoteToForeground()
97158
}
98159
null -> {
99160
// Service restarted by the system after process kill (START_STICKY).
100161
// Re-enter foreground immediately to prevent ANR, then notify host.
101162
Log.i(TAG, "Service restarted after process kill")
102-
startForeground(NOTIFICATION_ID, buildNotification())
103-
isRunning = true
163+
promoteToForeground()
104164
onServiceRestarted?.invoke()
105165
}
106166
}
@@ -109,10 +169,69 @@ class MeshForegroundService : Service() {
109169

110170
override fun onDestroy() {
111171
isRunning = false
172+
startRequested = false
112173
Log.i(TAG, "Mesh foreground service destroyed")
113174
super.onDestroy()
114175
}
115176

177+
/**
178+
* Enter (or re-enter) the foreground with the mesh notification.
179+
*
180+
* Shared by all three promotion sites so none of them can throw where the
181+
* caller does not expect it. A `connectedDevice`-typed promotion is not
182+
* failure-free on targetSdk 34: it raises SecurityException once the
183+
* Nearby-Devices runtime permissions are revoked, and a START_STICKY
184+
* restart while the app is backgrounded raises
185+
* ForegroundServiceStartNotAllowedException on Android 12+. Neither is
186+
* worth taking the process down for by itself, and the ACTION_START path
187+
* re-promotes afterwards.
188+
*
189+
* The residual is worth stating plainly: when the instance was created by
190+
* startForegroundService() and promotion genuinely fails, swallowing here
191+
* only defers the kill — the system still raises its own 5-second-timeout
192+
* RemoteServiceException. This buys survival for transient OEM failures,
193+
* not immunity.
194+
*/
195+
private fun promoteToForeground() {
196+
try {
197+
startForeground(NOTIFICATION_ID, buildNotification())
198+
isRunning = true
199+
} catch (e: Exception) {
200+
Log.w(TAG, "startForeground failed: ${e.message}", e)
201+
}
202+
}
203+
204+
/**
205+
* Hand a user-initiated stop to the host, which owns protocol and
206+
* transport lifecycle. Its teardown ends in [stop], so this service comes
207+
* down through the ACTION_STOP branch once the mesh is actually off —
208+
* keeping the notification truthful rather than clearing it while the
209+
* radios still run.
210+
*/
211+
private fun handleUserStopRequest(): Int {
212+
val callback = onStopRequestedByUser
213+
if (callback != null) {
214+
try {
215+
callback.invoke()
216+
return START_NOT_STICKY
217+
} catch (e: Exception) {
218+
Log.w(TAG, "Host stop callback failed; stopping the service directly: ${e.message}", e)
219+
}
220+
} else {
221+
Log.w(TAG, "No host stop callback registered; stopping the service only — transports may still be running")
222+
}
223+
// Fallback: nothing to defer to, so at least honour the button by
224+
// dropping the keep-alive.
225+
stopForegroundAndSelf()
226+
return START_NOT_STICKY
227+
}
228+
229+
private fun stopForegroundAndSelf() {
230+
isRunning = false
231+
stopForeground(STOP_FOREGROUND_REMOVE)
232+
stopSelf()
233+
}
234+
116235
private fun createNotificationChannel() {
117236
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
118237
val channel = NotificationChannel(
@@ -129,13 +248,38 @@ class MeshForegroundService : Service() {
129248
}
130249

131250
private fun buildNotification(): Notification {
251+
// Route the notification's stop action back through this service's
252+
// own ACTION_STOP_FROM_NOTIFICATION handler. Using
253+
// PendingIntent.getForegroundService on O+ keeps the delivery legal
254+
// under the background service-start restrictions that apply when the
255+
// user taps the action from the notification shade while the app
256+
// itself is in the background; pre-O falls back to getService which
257+
// has no such restriction.
258+
//
259+
// Note the coupling with the onCreate promotion: if this fires against
260+
// a dead service instance (a stale notification during a sticky-restart
261+
// gap), startForegroundService() re-creates the service, and that
262+
// promotion is the only thing satisfying the 5-second startForeground
263+
// obligation before the stop is handled. Do not remove one without
264+
// the other.
265+
val stopIntent = Intent(this, MeshForegroundService::class.java).apply {
266+
action = ACTION_STOP_FROM_NOTIFICATION
267+
}
268+
val pendingIntentFlags = PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE
269+
val stopPendingIntent = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
270+
PendingIntent.getForegroundService(this, 0, stopIntent, pendingIntentFlags)
271+
} else {
272+
PendingIntent.getService(this, 0, stopIntent, pendingIntentFlags)
273+
}
274+
132275
return NotificationCompat.Builder(this, CHANNEL_ID)
133276
.setContentTitle("Mesh Active")
134277
.setContentText("Offline mesh networking is running")
135278
.setSmallIcon(android.R.drawable.stat_sys_data_bluetooth)
136279
.setOngoing(true)
137280
.setPriority(NotificationCompat.PRIORITY_LOW)
138281
.setCategory(NotificationCompat.CATEGORY_SERVICE)
282+
.addAction(android.R.drawable.ic_media_pause, "Stop", stopPendingIntent)
139283
.build()
140284
}
141285
}

0 commit comments

Comments
 (0)