Skip to content

Commit 55a1f70

Browse files
adalpariclaude
andauthored
Fix UploadService foreground service timeout crash on Android 15+ (#23137)
* Fix dataSync foreground service timeout crash in UploadService onTimeout() called stopSelf(startId), which is a no-op unless startId is the most recent start. UploadService starts once per queued upload, so the id delivered on timeout is usually stale and the service never stopped, triggering ForegroundServiceDidNotStopInTimeException. Stop unconditionally and leave the foreground state immediately instead. Fixes WORDPRESS-ANDROID-38SC Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Guard notification counters against stale upload callbacks After a foreground-service timeout, handleForegroundServiceTimeout() resets the static notification counters synchronously while the cancellations it triggers report back asynchronously, corrupting the reset state. Ignore completion/cancel callbacks when nothing is being tracked (both totals zero), which is the only state that exists after a reset. Also dedup the cancel+reset block in the completion path via resetForegroundNotificationState(), and replace the deprecated stopForeground(true) with ServiceCompat.stopForeground(...). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 3884179 commit 55a1f70

2 files changed

Lines changed: 61 additions & 15 deletions

File tree

WordPress/src/main/java/org/wordpress/android/ui/uploads/PostUploadNotifier.java

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import androidx.annotation.Nullable;
1212
import androidx.collection.SparseArrayCompat;
1313
import androidx.core.app.NotificationCompat;
14+
import androidx.core.app.ServiceCompat;
1415
import androidx.core.app.TaskStackBuilder;
1516

1617
import org.greenrobot.eventbus.EventBus;
@@ -128,9 +129,7 @@ private void updateNotificationBuilder(@Nullable PostImmutableModel post) {
128129
}
129130

130131
private synchronized void startOrUpdateForegroundNotification(@Nullable PostImmutableModel post) {
131-
boolean isTotalPostsAndMediaItemsCountZero = sNotificationData.mTotalPostItems == 0
132-
&& sNotificationData.mTotalMediaItems == 0;
133-
if (isTotalPostsAndMediaItemsCountZero) {
132+
if (hasNoForegroundItemsTracked()) {
134133
return;
135134
}
136135
updateNotificationBuilder(post);
@@ -214,6 +213,12 @@ void incrementUploadedPostCountFromForegroundNotification(@NonNull PostImmutable
214213
}
215214

216215
void incrementUploadedPostCountFromForegroundNotification(@NonNull PostImmutableModel post, boolean force) {
216+
// Ignore stale completion callbacks that arrive after the foreground state was reset (e.g.
217+
// cancellations dispatched during a foreground-service timeout). With nothing being tracked,
218+
// counting them would push the current counters past the zeroed totals and corrupt the state.
219+
if (hasNoForegroundItemsTracked()) {
220+
return;
221+
}
217222
// first we need to check that we only count this post once as "ended" (either successfully or with error)
218223
// for every error we get. We'll then try to increment the Post count as it's been cancelled/failed because the
219224
// related media was cancelled or has failed too (i.e. we can't upload a Post with failed media, therefore
@@ -232,6 +237,11 @@ void incrementUploadedPostCountFromForegroundNotification(@NonNull PostImmutable
232237
}
233238

234239
void incrementUploadedMediaCountFromProgressNotification(int mediaId) {
240+
// Ignore stale completion callbacks that arrive after the foreground state was reset (see
241+
// incrementUploadedPostCountFromForegroundNotification above).
242+
if (hasNoForegroundItemsTracked()) {
243+
return;
244+
}
235245
sNotificationData.mCurrentMediaItem++;
236246
if (!removeNotificationAndStopForegroundServiceIfNoItemsInQueue()) {
237247
// update Notification now
@@ -242,16 +252,32 @@ void incrementUploadedMediaCountFromProgressNotification(int mediaId) {
242252
private boolean removeNotificationAndStopForegroundServiceIfNoItemsInQueue() {
243253
if (sNotificationData.mCurrentPostItem == sNotificationData.mTotalPostItems
244254
&& sNotificationData.mCurrentMediaItem == sNotificationData.mTotalMediaItems) {
245-
mNotificationManager.cancel(sNotificationData.mNotificationId);
246-
// reset the notification id so a new one is generated next time the service is started
247-
sNotificationData.mNotificationId = 0;
248-
resetNotificationCounters();
249-
mService.stopForeground(true);
255+
resetForegroundNotificationState();
256+
ServiceCompat.stopForeground(mService, ServiceCompat.STOP_FOREGROUND_REMOVE);
250257
return true;
251258
}
252259
return false;
253260
}
254261

262+
private boolean hasNoForegroundItemsTracked() {
263+
return sNotificationData.mTotalPostItems == 0 && sNotificationData.mTotalMediaItems == 0;
264+
}
265+
266+
/**
267+
* Clears the outstanding foreground notification and resets its state. Called both when all
268+
* uploads complete and when the service is force-stopped (e.g. a foreground-service timeout),
269+
* so the next upload starts a fresh foreground notification via startForeground().
270+
*/
271+
void resetForegroundNotificationState() {
272+
if (sNotificationData.mNotificationId != 0) {
273+
// cancel the outstanding notification and reset the id so a new one is generated next
274+
// time the service is started
275+
mNotificationManager.cancel(sNotificationData.mNotificationId);
276+
sNotificationData.mNotificationId = 0;
277+
}
278+
resetNotificationCounters();
279+
}
280+
255281
private void resetNotificationCounters() {
256282
sNotificationData.mCurrentPostItem = 0;
257283
sNotificationData.mCurrentMediaItem = 0;

WordPress/src/main/java/org/wordpress/android/ui/uploads/UploadService.java

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import androidx.annotation.NonNull;
1010
import androidx.annotation.Nullable;
1111
import androidx.annotation.RequiresApi;
12+
import androidx.core.app.ServiceCompat;
1213

1314
import org.greenrobot.eventbus.EventBus;
1415
import org.greenrobot.eventbus.Subscribe;
@@ -176,26 +177,45 @@ public int onStartCommand(Intent intent, int flags, int startId) {
176177
@Override
177178
public void onTimeout(int startId) {
178179
super.onTimeout(startId);
179-
stopSelf(startId);
180-
AppLog.i(T.MAIN, "UploadService > timed out");
180+
handleForegroundServiceTimeout();
181181
}
182182

183183
// Android 15+ enforces a 6-hour-per-24h cap on dataSync foreground services. The system calls
184-
// this two-arg overload before throwing ForegroundServiceDidNotStopInTimeException; we have a
185-
// few seconds to stop. Cancel in-flight uploads eagerly so onDestroy() finishes within budget.
184+
// this two-arg overload before throwing ForegroundServiceDidNotStopInTimeException.
186185
@Override
187186
@RequiresApi(Build.VERSION_CODES.VANILLA_ICE_CREAM)
188187
public void onTimeout(int startId, int fgsType) {
189188
super.onTimeout(startId, fgsType);
190-
AppLog.w(T.MAIN, "UploadService > foreground service timeout reached (type=" + fgsType
191-
+ "); stopping service");
189+
AppLog.w(T.MAIN, "UploadService > foreground service timeout reached (type=" + fgsType + ")");
190+
handleForegroundServiceTimeout();
191+
}
192+
193+
/**
194+
* Stops the service after the system signals a foreground-service timeout, in the few seconds we
195+
* have before {@link android.app.RemoteServiceException.ForegroundServiceDidNotStopInTimeException}
196+
* is thrown.
197+
*
198+
* We intentionally do NOT use {@code stopSelf(startId)} here: that overload only stops the service
199+
* when {@code startId} matches the most recent start, but the service is started once per queued
200+
* upload, so the id delivered to onTimeout() is usually stale and the call would be a no-op. We
201+
* also leave the foreground state right away rather than relying on onDestroy() finishing within
202+
* the timeout budget.
203+
*/
204+
private void handleForegroundServiceTimeout() {
192205
if (mMediaUploadHandler != null) {
193206
mMediaUploadHandler.cancelInProgressUploads();
194207
}
195208
if (mPostUploadHandler != null) {
196209
mPostUploadHandler.cancelInProgressUploads();
197210
}
198-
stopSelf(startId);
211+
// Leave the foreground state immediately so the system's timeout window is satisfied, and
212+
// reset the notifier so the next upload re-enters the foreground via startForeground().
213+
if (mPostUploadNotifier != null) {
214+
mPostUploadNotifier.resetForegroundNotificationState();
215+
}
216+
ServiceCompat.stopForeground(this, ServiceCompat.STOP_FOREGROUND_REMOVE);
217+
stopSelf();
218+
AppLog.i(T.MAIN, "UploadService > stopped after foreground service timeout");
199219
}
200220

201221
private void unpackMediaIntent(@NonNull Intent intent) {

0 commit comments

Comments
 (0)