Skip to content

feat(library): make LibraryLoader/DocumentLoader Send on native targets#71

Open
bkfunk wants to merge 1 commit into
segfault87:mainfrom
bkfunk:feature/send-compat-loader
Open

feat(library): make LibraryLoader/DocumentLoader Send on native targets#71
bkfunk wants to merge 1 commit into
segfault87:mainfrom
bkfunk:feature/send-compat-loader

Conversation

@bkfunk
Copy link
Copy Markdown

@bkfunk bkfunk commented May 13, 2026

Summary

LibraryLoader and DocumentLoader (in ldraw/src/library.rs) are declared with #[async_trait(?Send)]. That ?Send propagates through both native resolvers (LocalLoader, HttpLoader) and through resolve_dependencies_multipart.

On wasm32 this is necessary — wasm-bindgen futures are deliberately !Send because the web platform is single-threaded. But on native, the underlying work is already Send:

  • LocalLoader uses tokio::fs::File / BufReader / try_exists — all Send.
  • HttpLoader uses reqwest::Client futures — also Send.
  • The shared PartCache lives in Arc<RwLock<...>> (already Send + Sync).

The ?Send was the only thing forcing native consumers running on a multi-threaded tokio runtime (e.g. Tauri's default) into ceremonies like:

tokio::task::spawn_blocking(move || {
    let rt = tokio::runtime::Builder::new_current_thread().enable_all().build()?;
    rt.block_on(async move { loader.load(&part_id).await })
}).await

A fresh runtime is constructed on every call. That's fine for one-shot loads but adds up for thumbnail-heavy UIs that batch dozens of part loads.

Change

Switch the four #[async_trait(?Send)] attributes (the two traits in library.rs and the four impls across resolvers/local.rs and resolvers/http.rs) to the standard cfg-gated pattern:

#[cfg_attr(not(target_arch = "wasm32"), async_trait)]
#[cfg_attr(target_arch = "wasm32", async_trait(?Send))]

On native, futures returned by LibraryLoader::load_ref, LibraryLoader::load_colors, and DocumentLoader::load_document are now Send. On wasm32, the existing ?Send behavior is preserved.

Compile-time Send regression guard

Adds dead helper functions in each resolver whose bodies call assert_send::<F>(_) on the futures returned by load_ref and load_colors. These never run, but the compiler type-checks their bodies, so any future regression that makes the futures !Send on native will fail compilation.

Test plan

  • cargo check --workspace passes on native
  • cargo check -p ldraw --target wasm32-unknown-unknown passes
  • cargo test -p ldraw --lib passes

`LibraryLoader` and `DocumentLoader` were declared with
`#[async_trait(?Send)]`, which propagated `?Send` through every
resolver and through `resolve_dependencies*`. On wasm32 that's
necessary (wasm-bindgen futures are deliberately `!Send` because the
web platform is single-threaded), but on native it forced consumers
running on a multi-threaded tokio runtime (e.g. Tauri's default) into
ceremonies like `spawn_blocking` + `new_current_thread`, paying a
fresh-runtime construction on every part load.

The underlying work is already `Send` on native: `LocalLoader` uses
`tokio::fs` and `BufReader` (both `Send`), `HttpLoader` uses
`reqwest::Client` futures (`Send`), and `PartCache` lives in
`Arc<RwLock<...>>` (already `Send + Sync`). The `?Send` was the only
thing forcing the worse codepath.

Switch the four `#[async_trait(?Send)]` attributes (the two traits in
`library.rs` and the four impls across the two resolvers) to:

    #[cfg_attr(not(target_arch = "wasm32"), async_trait)]
    #[cfg_attr(target_arch = "wasm32", async_trait(?Send))]

This is the standard pattern for libraries that target both platforms.
Native consumers can now `.await` `LibraryLoader` futures directly on
worker threads; the wasm32 build is unaffected.

Adds compile-time `Send` checks in both resolvers: dead helper
functions whose bodies call `assert_send::<F>(_)` on the future
returned by `load_ref`/`load_colors`. These don't run at runtime, but
will fail to compile if the futures ever lose `Send` again.

Verified:
- `cargo check --workspace` passes on native
- `cargo check -p ldraw --target wasm32-unknown-unknown` passes
- `cargo test -p ldraw --lib` passes

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Copilot AI review requested due to automatic review settings May 13, 2026 18:46
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes the ?Send constraint on LibraryLoader and DocumentLoader traits (and their LocalLoader/HttpLoader impls) on native targets, while preserving ?Send on wasm32 where it is still required. This lets callers running on a multi-threaded tokio runtime (e.g. Tauri) .await these futures directly across threads instead of constructing a per-call current-thread runtime inside spawn_blocking.

Changes:

  • Switch four #[async_trait(?Send)] attributes to a cfg_attr pair that uses the plain #[async_trait] (Send) on non-wasm32 and #[async_trait(?Send)] on wasm32, applied to both traits in library.rs and the four impls in resolvers/local.rs and resolvers/http.rs.
  • Add per-resolver dead _assert_*_futures_are_send helpers gated to non-wasm targets that pass the futures returned by load_ref and load_colors to a generic assert_send::<F: Send> to provide a compile-time regression guard.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
ldraw/src/library.rs Replaces ?Send on the DocumentLoader and LibraryLoader trait declarations with target-gated async_trait attributes.
ldraw/src/resolvers/local.rs Same gating on the two LocalLoader impls; adds a non-wasm _assert_localloader_futures_are_send compile-time check.
ldraw/src/resolvers/http.rs Same gating on the two HttpLoader impls; adds a parallel non-wasm _assert_httploader_futures_are_send compile-time check.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.

2 participants