Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Remove Send bounds on wasm32 #2074

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

OliverEvans96
Copy link
Contributor

@OliverEvans96 OliverEvans96 commented Sep 2, 2024

Fixes #910

Also related: #1235

Changes

Attempting to compile to WASM produces a number of errors regarding Send bounds, as reported in #1235. This is because wasm_bindgen::JsValue is not Send, since it's intended to run in a single-threaded JS environment.

This issue was successfully addressed in the rspotify crate in ramsayleung/rspotify#458, although I think opentelemetry-rust requires a somewhat more complex solution.

I've managed to fix these errors (see other WASM errors in #1472 and #2047) by conditionally removing Send bounds for target_arch="wasm32".

I've done this in three ways:

  • conditionally change #[async_trait] to #[async_trait(?Send)] based on target_arch
  • Replace BoxFuture with MaybeSendBoxFuture, which conditionally removes Send bounds on wasm
  • Replace Arc<dyn InstrumentProvider + Send + Sync> with type ArcInstrumentProvider = Arc<dyn InstrumentProvider> on WASM and type ArcInstrumentProvider = Arc<dyn InstrumentProvider + Send + Sync> otherwise.

I suspect that this PR is not yet complete. Initially, I only made enough changes to get my application to compile, using the following feature flags:

opentelemetry = { version = "0.24.0" }
opentelemetry_sdk = { version = "0.24.1", features=["logs"] }
opentelemetry-semantic-conventions = "0.16.0"
opentelemetry-otlp = { version = "0.17.0", default-features = false, features = ["logs", "trace", "http", "http-json", "reqwest-client"] }
opentelemetry-http = { version = "0.13.0", default-features = false }

I then continued by replacing all instances of #[async_trait] that seemed relevant. But I assume that there are other crates or feature flags that would require similar changes.

I'm also happy to hear your feedback on this approach, and make changes as necessary.

Thanks,
Oliver

Merge requirement checklist

  • CONTRIBUTING guidelines followed
  • Unit tests added/updated (if applicable)
  • Appropriate CHANGELOG.md files updated for non-trivial, user-facing changes
  • Changes in public API reviewed (if applicable)

@OliverEvans96 OliverEvans96 requested a review from a team September 2, 2024 15:43
@OliverEvans96
Copy link
Contributor Author

I'm also wondering whether it's really wise to assume that all wasm32 targets are single-threaded. Are there or will there be multi-threaded WASM environments??

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

*mut u8` cannot be sent between threads safely async_std
1 participant