Fix subscription model encoding and notification races

[chunhuidu-dev]

Description

One Line Summary

Prevents concurrent OSSubscriptionModel mutations from crashing during archival or emitting stale, out-of-order subscription updates.

Details

Motivation

Fixes #1588.

OSSubscriptionModel.encode(with:) previously read mutable reference fields while other queues could replace them. Archiving could therefore race ARC retain/release operations and crash in objc_retain.

State writes and set(property:newValue:) notifications were also separate operations. Concurrent setters could update the model in one order but enqueue deltas in another order. Push subscription observers also rebuilt their snapshots from live state, which could combine values from different mutations.

Scope

• Store mutable subscription fields in a lock-protected value-type state.
• Archive and serialize one atomic state snapshot.
• Serialize each mutation together with its model delta and push observer notification.
• Build push subscription previous and current values from paired mutation snapshots.
• Add deterministic concurrency coverage for archival snapshots and mutation notification ordering.
• No public API changes.

Testing

Unit testing

Added deterministic tests that:

• Pause keyed archiving at the address field, mutate the model concurrently, then decode and verify the archive contains one consistent snapshot.
• Hold the first address notification while confirming a second setter is actually waiting on the mutation lock, then verify notifications remain ordered.

Validation performed on an iPhone 16 simulator running iOS 26.5:

• OneSignalUserTests build-for-testing succeeded.
• testEncodingSubscriptionModel_withConcurrentMutation_usesConsistentSnapshot passed.
• testConcurrentSubscriptionChangesNotifyInMutationOrder passed.

Manual testing

Physical-device testing was not performed because this is an internal synchronization fix covered by deterministic unit tests.

Affected code checklist

• Notifications
  • Display
  • Open
  • Push Processing
  • Confirm Deliveries
• Outcomes
• Sessions
• In-App Messaging
• REST API requests
• Public API changes

Checklist

Overview

• I have filled out all required sections above.
• This PR addresses one concurrency issue and its regression coverage.
• There are no public API changes.

Testing

• I have included test coverage for these changes.
• The relevant targeted build and automated tests pass; the full suite was not run because the change is scoped to OSSubscriptionModel concurrency.
• The lack of physical-device testing is explained above.

Final pass

• The code is as readable as possible.
• I reviewed the complete PR diff.

[nan-li]

@claude review

[claude]

No bugs found, but this is a nontrivial concurrency rewrite of the push/email/SMS subscription model's internal state (new lock-protected state struct, atomic archiving snapshot, and reordered mutation/notification semantics), so it's worth a human look given how central this model is to push token syncing.

What was reviewed: the new stateLock/mutationLock pairing for deadlock risk (no nested non-reentrant lock acquisitions found), the notificationTypes/_isDisabled interaction that a finder flagged (confirmed state still converges to -2/reachable=false, and the skipped self.set only happens on a no-op value bounce, matching what verifiers ruled out), and the _isDisabled setter's two-stage firePushSubscriptionChanged calls against the original didSet recursion (semantics match).

Extended reasoning...

Overview

The PR reworks OSSubscriptionModel (push/email/SMS subscription model) to store all mutable fields in a single lock-protected State struct, with a non-reentrant stateLock guarding field reads/writes and a separate (default recursive) mutationLock serializing each full setter operation (mutation + delta enqueue + push-observer notification). encode(with:) now archives one atomic snapshot instead of reading live fields one at a time. Test coverage adds two deterministic concurrency tests: one pausing keyed archiving mid-encode to prove snapshot consistency, and one proving notification ordering under lock contention using a custom condition-based recursive lock.

Security risks

None identified. This is purely internal synchronization / state management; no new user input handling, network calls, or auth logic.

Level of scrutiny

This warrants more than a cursory pass despite zero reported bugs: it changes locking primitives and mutation ordering in a core model that backs the push subscription (token, ids, opt-in state) synced to the backend, and a subtle mistake here could reproduce the exact class of race/crash bug it is fixing (per the linked issue #1588). I traced the lock usage for reentrancy/deadlock risk and manually replayed the notificationTypes/_isDisabled interaction that finder agents flagged, and both held up, but the change is large and central enough that a bot approval shouldn't substitute for a human pass, especially given the intricate snapshot-patching logic in the firePushSubscriptionChanged call sites.

Other factors

The PR includes two well-targeted deterministic concurrency tests that reproduce the original crash/ordering scenarios rather than relying on flaky stress tests, which increases confidence in the fix. No public API changes per the description, and the diff is scoped to a single file plus its tests.

[chunhuidu-dev]

I pushed a follow-up commit to resolve the SwiftLint type_body_length failure by moving the subscription metadata properties into an extension. This is a structural refactor only and does not change behavior. The two targeted concurrency tests pass locally.