fix(swift-sdk): serialize SwiftData ModelContext access from FFI callbacks#3558
Conversation
…backs
`PlatformWalletPersistenceHandler` shared a single `ModelContext`
across FFI callback shims, but `ModelContext` is not thread-safe.
The Rust persister fires its callbacks on Tokio worker threads, so
multiple persistence callbacks could (and did) hit `fetch`/`save`
on the same context concurrently, corrupting SwiftData's internal
state and crashing inside `ModelContext.fetch` — typically inside
`markUtxoSpent` mid-changeset.
Confine the context to a serial `DispatchQueue` and route every
public entry point through an `onQueue { … }` helper that calls
`queue.sync(execute:)`. The synchronous bracketing matches the
synchronous FFI contract (each shim still returns its `Int32`
status before yielding back to Rust) and gives the handler the
same isolation guarantees as an actor without forcing the FFI
boundary to become async.
Two cache-loader methods (`loadCachedBalances`,
`loadCachedSyncState(network:)`) had to be split into a public
queue-wrapping wrapper plus a private `*OnQueue` impl so
`loadWalletList` — already on the queue — can call them without
re-entering `sync` and deadlocking. The `loadCachedSyncState(walletId:)`
overload now also routes through the on-queue impl after resolving
the network in a single hop.
`loadAllocations` (the dictionary that retains the heap buffers
returned to Rust) is also confined to the queue, so the
`loadWalletList` insert and the `loadWalletListFree` remove no
longer race when Rust drives them on different worker threads.
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Review GateCommit:
|
|
✅ DashSDKFFI.xcframework built for this PR.
SwiftPM (host the zip at a stable URL, then use): .binaryTarget(
name: "DashSDKFFI",
url: "https://your.cdn.example/DashSDKFFI.xcframework.zip",
checksum: "7045764bb0699a08a8c8ad3a58818322e4044ac8dd81cfe59607d169d78087cb"
)Xcode manual integration:
|
…elcontext-thread-safety
…AddressesSyncState The model only stores the BLAST address-sync watermark — its name read as if it were a generic sync state. Rename the @model class, its backing file, the two storage-explorer views, and the storage- explorer row label to make the scope explicit. Pure rename across the SwiftDashSDK target and the example app: no behavior change, no `@Model(originalName:)` migration shim (existing rows on the old table are orphaned — acceptable, the next sync round repopulates).
2 participants
Summary
PlatformWalletPersistenceHandlershares a single SwiftDataModelContextacross the FFI callback shims that the Rust persister invokes — butModelContextis not thread-safe, and those shims execute on Tokio worker threads. Concurrentfetch/savecorrupts SwiftData's internal state and crashes insideModelContext.fetch, e.g.:The Tokio runtime is fanning multiple persistence callbacks (begin/changeset/identities/keys/end, plus
loadWalletList↔loadWalletListFreepairs) at the same context; the crash above lands mid-utxos_spentbecause that's a frequent fetch-then-mutate path inside the changeset round.Fix
Confine
backgroundContext(and theloadAllocationsdictionary backingloadWalletList/loadWalletListFree) to a private serial `DispatchQueue`. Every public entry point now wraps its body inonQueue { … }, which callsqueue.sync(execute:):Int32status before yielding back to Rust, no async bridging needed.@ModelActorwithout forcing the FFI boundary to become async.upsertTransaction,markUtxoSpent,applyAccountChangeset,walletNetwork,ensureWalletRecord,linkTokenBalanceRelations,deriveAndStoreIdentityKey, …) are unchanged: they assume they're already on the queue, which holds because they're only reachable from a wrapped public method.Two cache loaders had to be split into public wrapper + private
*OnQueueimpl soloadWalletList— itself on the queue — can call them without re-entering `sync` and deadlocking:loadCachedBalances(walletId:)→loadCachedBalancesOnQueue(walletId:)loadCachedSyncState(network:)→loadCachedSyncStateOnQueue(network:)(the(walletId:)overload now resolves the network and reads the row in a single queue hop via the on-queue impl)`loadAllocations` is also covered: `loadWalletList` writes the entry pointer into the dict on the queue, `loadWalletListFree` removes it on the queue, so they no longer race when Rust drives them on different worker threads.
Why not
@ModelActor?Tried first; an actor forces every callback to bridge sync → async, which the Rust persister's
Int32-return contract makes painful (semaphore + wake-blocking on the worker thread). The serial queue is the same isolation in shape, with no FFI signature changes and no actor-hop between intra-changeset calls.Test plan
swiftc -typecheckover the SwiftDashSDK target with the iOS-simulator slice + Swift 6 strict concurrency passesutxos_spententries) on a build without this fix, then re-run with the fix and confirm noModelContextcrashchangesetBegin→ manypersist*callbacks →changesetEnd) still commit atomically — the per-round bracketing is unchanged, just serialized🤖 Generated with Claude Code