fix(wasm): relax networking traits Send+Sync via MaybeSendSync - #795
Merged
Conversation
…MaybeSendSync The three networking traits in wacore/src/net.rs hardcoded a Send + Sync supertrait, forcing a wasm32 transport/HTTP implementation to be Send + Sync even though the client is intentionally !Send there and a browser WebSocket/fetch backend holds !Send JS handles. Mirror the established convention (EventHandler, SendContextResolver, EncHandler): supertrait becomes crate::sync_marker::MaybeSendSync, which is Send + Sync on native (the blanket impl keeps Arc<dyn Transport>/Arc<dyn HttpClient> cross-thread storage unchanged) and no bound on wasm32. The async_trait(?Send) gates already existed and are now consistent with the relaxed supertrait. Verified native build/clippy and the wasm32 lib build. Note: the sibling store/Backend traits (portability-05) are NOT included here. That relaxation cascades into 128 errors because impl_store_wrapper and the Device hold Arc<dyn Backend> and require it to be Send + Sync; relaxing those needs the wrapper macro and Device made wasm-aware too, which is a separate, larger change.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThree networking traits in ChangesNetworking trait sync bound relaxation
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Benchmark Results67 unchanged benchmark(s)
|
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The three networking traits in
wacore/src/net.rs(Transport,TransportFactory,HttpClient) hardcoded aSend + Syncsupertrait, forcing a wasm32 transport/HTTP implementation to beSend + Synceven though the client is intentionally!Sendthere and a browser WebSocket/fetch backend holds!SendJS handles.This mirrors the established convention (
EventHandler,SendContextResolver, and the recently-mergedEncHandlerin #793): the supertrait becomescrate::sync_marker::MaybeSendSync, which isSend + Syncon native and no bound on wasm32. Theasync_trait(?Send)gates already existed and are now consistent with the relaxed supertrait.Why
Unblocks
!Send-handle-backed transport/HTTP implementations on the wasm port.Native impact
None. The blanket
MaybeSendSyncimpl keepsdyn Transport/dyn HttpClientSend + Syncon native, so theArc<dyn Transport>/Arc<dyn HttpClient>cross-thread storage is unchanged.Scope note
This intentionally does not include the sibling store/
Backendtraits (portability-05). I tried it and that relaxation cascades into 128 errors:impl_store_wrapperand theDeviceholdArc<dyn Backend>and require it to beSend + Sync, so relaxing the store traits needs the wrapper macro andDevicemade wasm-aware too. That is a separate, larger change, not a one-line supertrait swap.Verification
Native build +
cargo clippy --all-targets -- -D warningsclean, and the wasm32 lib build (cargo build -p whatsapp-rust --lib --release --target wasm32-unknown-unknown --no-default-features --features debug-diagnostics) passes.