Fix synchronization issue with android notification by thomasttvo · Pull Request #21 · openspacelabs/react-native-background-upload

thomasttvo · 2025-10-17T00:54:03Z

Fixes notification race condition where queued uploads could overwrite the active upload's notification settings. The bug occurred because each Upload object carried its own notification config—when a Wi-Fi-only upload was queued while a regular upload was running, the queued upload could touch the shared system notification and apply its wifiOnly=true preference before actually executing.

Centralization solves this by making UploadNotification the single source of truth for notification state. It tracks exactly one activeUpload at a time (synchronized), and notification content is built by reading getActiveUpload()?.wifiOnly rather than arbitrary upload configs. Queued uploads can exist in memory with their settings, but they can't affect the notification until they call setActiveUpload() when they start executing. This eliminates the race condition—only the actively running upload determines what the user sees.

Test Plan:

Start a regular upload (non-Wi-Fi-only)
While it's running, queue a Wi-Fi-only upload
Verify notification shows correct connectivity preference for the actively running upload (not the queued one)
Wait for first upload to complete, verify notification updates to reflect the now-active Wi-Fi-only upload
Test notification persistence across app kills and device restarts
Verify both uploads complete successfully

Note

Android notification handling consolidated and race fixed

Introduces UploadNotification singleton and Connectivity.fetch to build/update a single foreground notification reflecting active upload or queue needs (WiFi vs internet), with shared maxRetries
Removes per-upload Android notification options from Upload; adds initialize(options) to set global notification config (JS/types updated)
Refactors UploadWorker to use centralized notification/ connectivity, mark active upload, and update notification on progress/success/error; retries now use global maxRetries
Enhances UploadProgress to track wifiOnly and expose hasNonWifiOnlyUploads() for accurate notification messages
Updates example app to call Upload.initialize and starts multiple uploads; bumps package to 7.6.0

^{Written by Cursor Bugbot for commit c2c9922. Configure here.}

thomasttvo · 2025-10-17T00:54:34Z

android/src/main/java/com/vydia/RNUploader/Connectivity.kt

+  NoWifi, NoInternet, Ok;
+
+  companion object {
+    fun fetch(context: Context, wifiOnly: Boolean): Connectivity {


moved from UploadWorker.ts no logical change

thomasttvo · 2025-10-17T00:55:13Z

android/src/main/java/com/vydia/RNUploader/Notification.kt

+  }
+
+  // builds the notification required to enable Foreground mode
+  fun build(context: Context): Notification {


moved from UploadWorker.ts. Not much has changed except for val wifiOnly = getActiveUpload()?.wifiOnly ?: false

thomasttvo · 2025-10-17T00:56:01Z

android/src/main/java/com/vydia/RNUploader/Notification.kt

+    if (this.activeUpload?.id == upload.id) this.activeUpload = null
+  }
+
+  fun setOptions(opts: ReadableMap) {


these options are moved over from the Upload class, so they are now global options instead of per-Upload options

elliottkember

LGTM

dhruv-dangi-openspace · 2025-10-30T14:09:56Z

example/RNBGUExample/App.tsx

-        setProgress(undefined);
-        console.log('Upload error!', err);
-      });
+    for (let i = 0; i < 100; i++) {


Can we add a comment explaining the magic number 100 and why we have to perform Upload.startUpload 100 times?

ah this is just the test app, but it's good to add a comment still

- Name response variable in use block (UploadUtils.kt) - Fix shadowed it in takeIf lambda (UploadUtils.kt) - Rename set() to setIfNotNull() (UploadProgress.kt) - Add complete() method to Progress class (UploadProgress.kt) - Rename clearIfNeeded() to clearIfCompleted() (UploadProgress.kt) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

…gress

…fication

thomasttvo · 2026-01-21T01:45:19Z

bugbot run

Ensures the notification shows correct "Waiting for WiFi/Internet" title when network degrades after initial foreground setup. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

thomasttvo · 2026-01-22T01:57:30Z

bugbot run

cursor

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.}

Comment @cursor review or bugbot run to trigger another review on this PR

cursor · 2026-01-22T02:08:58Z

android/src/main/java/com/vydia/RNUploader/Notification.kt

+    channel = opts.getString("notificationChannel")
+      ?: throw MissingOptionException("notificationChannel")
+    maxRetries = if (opts.hasKey("maxRetries")) opts.getInt("maxRetries") else 5
+  }


Thread-safety gap in setOptions vs concurrent readers

Medium Severity

The setOptions() method writes to shared properties (id, title, channel, maxRetries, etc.) without @Synchronized, while build(), getForegroundInfo(), and checkRetry() read these properties from background worker threads. This creates a race condition where a worker resuming from a previous session could call build() while setOptions() is executing, resulting in inconsistent notification content (e.g., old channel ID paired with new title). Other methods in UploadNotification that access activeUpload are correctly synchronized, but this synchronization pattern isn't applied to the configuration properties.

Additional Locations (1)

android/src/main/java/com/vydia/RNUploader/Notification.kt#L59-L75

cursor · 2026-01-22T02:08:58Z

android/src/main/java/com/vydia/RNUploader/Upload.kt

  val url: String,
  val path: String,
  val method: String,
-  val maxRetries: Int,


Resumed workers use uninitialized notification settings after app restart

Medium Severity

The notification settings (notificationChannel, notificationId, etc.) were removed from the Upload class, so they're no longer serialized with WorkManager input data. When workers resume after an app restart (process death), they depend on UploadNotification singleton state which hasn't been initialized yet—initialize() is called by JS code that may not have executed. The worker uses the default channel "File Uploads" which likely doesn't exist, causing the foreground notification to fail. This could result in missing notifications or setForeground() failures that prevent the worker from running properly.

Additional Locations (1)

android/src/main/java/com/vydia/RNUploader/Notification.kt#L13-L26

thomasvo added 9 commits October 16, 2025 15:33

fix

a15c885

doc

8d3d074

fix

710a9ca

simpler

a3ac03b

fix

e3c135c

fix

67b6944

fix

0467b42

fix

6f402f8

fix

99c0f34

thomasttvo commented Oct 17, 2025

View reviewed changes

thomasttvo requested a review from dhruv-dangi-openspace October 17, 2025 00:56

elliottkember approved these changes Oct 29, 2025

View reviewed changes

dhruv-dangi-openspace requested changes Oct 30, 2025

View reviewed changes

thomasvo and others added 6 commits December 11, 2025 16:42

set

28382b4

package

ca7748e

Merge remote-tracking branch 'origin/master' into thomas/simplify-pro…

c55c82c

…gress

Merge branch 'thomas/simplify-progress' into thomas/fix-unsynced-noti…

1e6d803

…fication

doc

d4bef0b

thomasttvo changed the base branch from thomas/simplify-progress to master January 21, 2026 01:41

bump

7063263

thomasttvo requested a review from dhruv-dangi-openspace January 21, 2026 01:42

This comment was marked as outdated.

Sign in to view

thomasvo and others added 4 commits January 21, 2026 15:02

semaphore fix

b94890f

maxretries

874935a

Update notification when connectivity check fails

488717f

Ensures the notification shows correct "Waiting for WiFi/Internet" title when network degrades after initial foreground setup. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

better default wifiOnly

c2c9922

thomasttvo force-pushed the thomas/fix-unsynced-notification branch from cbc41c0 to c2c9922 Compare January 22, 2026 01:53

cursor bot reviewed Jan 22, 2026

View reviewed changes

thomasvo added 2 commits January 23, 2026 09:44

synchronized

7bc5b7c

persist notification options

e2cd58e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

Fix synchronization issue with android notification#21

Fix synchronization issue with android notification#21
thomasttvo wants to merge 22 commits intomasterfrom
thomas/fix-unsynced-notification

thomasttvo commented Oct 17, 2025 •

edited by cursor bot

Loading

Uh oh!

thomasttvo Oct 17, 2025

Uh oh!

thomasttvo Oct 17, 2025

Uh oh!

thomasttvo Oct 17, 2025

Uh oh!

elliottkember left a comment

Uh oh!

dhruv-dangi-openspace Oct 30, 2025

Uh oh!

thomasttvo Jan 21, 2026

Uh oh!

thomasttvo commented Jan 21, 2026

Uh oh!

This comment was marked as outdated.

Uh oh!

thomasttvo commented Jan 22, 2026

Uh oh!

cursor bot left a comment

Uh oh!

cursor bot Jan 22, 2026

Uh oh!

cursor bot Jan 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

Conversation

thomasttvo commented Oct 17, 2025 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thomasttvo Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

thomasttvo Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

thomasttvo Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

elliottkember left a comment

Choose a reason for hiding this comment

Uh oh!

dhruv-dangi-openspace Oct 30, 2025

Choose a reason for hiding this comment

Uh oh!

thomasttvo Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

thomasttvo commented Jan 21, 2026

Uh oh!

This comment was marked as outdated.

Uh oh!

thomasttvo commented Jan 22, 2026

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor bot Jan 22, 2026

Choose a reason for hiding this comment

Thread-safety gap in setOptions vs concurrent readers

Uh oh!

cursor bot Jan 22, 2026

Choose a reason for hiding this comment

Resumed workers use uninitialized notification settings after app restart

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

thomasttvo commented Oct 17, 2025 •

edited by cursor bot

Loading