Skip to content

Commit 046d1e5

Browse files
authored
[BEEEP] Add ThreadBoundRunner (#258)
## 🎟️ Tracking <!-- Paste the link to the Jira or GitHub issue or otherwise describe / point to where this change is coming from. --> ## 📔 Objective Introduce ThreadBoundRunner, a utility that owns a non-Send state pinned to a single thread. It exposes a Send API (`run_in_thread`) for submitting async tasks that operate on this state. Tasks are dispatched in order and run on the owning thread, enabling safe cross-thread interaction with thread-bound resources. Or in other words, it's a utility that allows us to easily depend on JavaScript objects across the WASM divide in the rest of our core infrastructure without having to worry about the single-threaded nature of WASM/JavaScript. ## ⏰ Reminders before review - Contributor guidelines followed - All formatters and local linters executed and passed - Written new unit and / or integration tests where applicable - Protected functional changes with optionality (feature flags) - Used internationalization (i18n) for all UI strings - CI builds passed - Communicated to DevOps any deployment requirements - Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team ## 🦮 Reviewer guidelines <!-- Suggested interactions but feel free to use (or not) as you desire! --> - 👍 (`:+1:`) or similar for great changes - 📝 (`:memo:`) or ℹ️ (`:information_source:`) for notes or general info - ❓ (`:question:`) for questions - 🤔 (`:thinking:`) or 💭 (`:thought_balloon:`) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion - 🎨 (`:art:`) for suggestions / improvements - ❌ (`:x:`) or ⚠️ (`:warning:`) for more significant problems or concerns needing attention - 🌱 (`:seedling:`) or ♻️ (`:recycle:`) for future improvements or indications of technical debt - ⛏ (`:pick:`) for minor or nitpick changes
1 parent af7ae90 commit 046d1e5

File tree

13 files changed

+507
-1
lines changed

13 files changed

+507
-1
lines changed

.cargo/config.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ rustflags = ["--cfg", "aes_armv8"]
66

77
[target.wasm32-unknown-unknown]
88
rustflags = ['--cfg', 'getrandom_backend="wasm_js"']
9+
runner = 'wasm-bindgen-test-runner'
910

1011
# Enable support for 16k pages on Android, JNA is using these same flags
1112
# https://android-developers.googleblog.com/2024/08/adding-16-kb-page-size-to-android.html

.github/workflows/rust-test.yml

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,37 @@ jobs:
4545
- name: Test
4646
run: cargo test --workspace --all-features
4747

48+
test-wasm:
49+
name: WASM
50+
runs-on: ubuntu-24.04
51+
52+
steps:
53+
- name: Checkout
54+
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
55+
56+
- name: Set Rust Toolchain
57+
id: toolchain
58+
shell: bash
59+
run: |
60+
RUST_TOOLCHAIN="$(grep -oP '^channel.*"(\K.*?)(?=")' rust-toolchain.toml)"
61+
echo "RUST_TOOLCHAIN=${RUST_TOOLCHAIN}" | tee -a "${GITHUB_OUTPUT}"
62+
63+
- name: Install rust
64+
uses: dtolnay/rust-toolchain@56f84321dbccf38fb67ce29ab63e4754056677e0 # stable
65+
with:
66+
toolchain: "${{ steps.toolchain.outputs.RUST_TOOLCHAIN }}"
67+
targets: wasm32-unknown-unknown
68+
components: rust-src
69+
70+
- name: Cache cargo registry
71+
uses: Swatinem/rust-cache@f0deed1e0edfc6a9be95417288c0e1099b1eeec3 # v2.7.7
72+
73+
- name: Install wasm-bindgen-cli
74+
run: cargo install wasm-bindgen-cli --version 0.2.100
75+
76+
- name: Test WASM
77+
run: cargo test --target wasm32-unknown-unknown -p bitwarden-wasm-internal -p bitwarden-threading -p bitwarden-error --all-features
78+
4879
coverage:
4980
name: Coverage
5081
runs-on: ubuntu-24.04

Cargo.lock

Lines changed: 19 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ bitwarden-fido = { path = "crates/bitwarden-fido", version = "=1.0.0" }
2929
bitwarden-generators = { path = "crates/bitwarden-generators", version = "=1.0.0" }
3030
bitwarden-ipc = { path = "crates/bitwarden-ipc", version = "=1.0.0" }
3131
bitwarden-send = { path = "crates/bitwarden-send", version = "=1.0.0" }
32+
bitwarden-threading = { path = "crates/bitwarden-threading", version = "=1.0.0" }
3233
bitwarden-sm = { path = "bitwarden_license/bitwarden-sm", version = "=1.0.0" }
3334
bitwarden-ssh = { path = "crates/bitwarden-ssh", version = "=1.0.0" }
3435
bitwarden-vault = { path = "crates/bitwarden-vault", version = "=1.0.0" }
@@ -63,6 +64,7 @@ validator = { version = ">=0.18.1, <0.20", features = ["derive"] }
6364
wasm-bindgen = { version = ">=0.2.91, <0.3", features = ["serde-serialize"] }
6465
js-sys = { version = ">=0.3.72, <0.4" }
6566
wasm-bindgen-futures = "0.4.41"
67+
wasm-bindgen-test = "0.3.45"
6668

6769
# There is an incompatibility when using pkcs5 and chacha20 on wasm builds. This can be removed once a new
6870
# rustcrypto-formats crate version is released since the fix has been upstreamed.

crates/bitwarden-error/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,4 +34,4 @@ workspace = true
3434
[dev-dependencies]
3535
serde.workspace = true
3636
trybuild = "1.0.101"
37-
wasm-bindgen-test = "0.3.45"
37+
wasm-bindgen-test = { workspace = true }
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
[target.wasm32-unknown-unknown]
2+
runner = 'wasm-bindgen-test-runner'

crates/bitwarden-threading/Cargo.toml

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
[package]
2+
name = "bitwarden-threading"
3+
version.workspace = true
4+
authors.workspace = true
5+
edition.workspace = true
6+
rust-version.workspace = true
7+
homepage.workspace = true
8+
repository.workspace = true
9+
license-file.workspace = true
10+
keywords.workspace = true
11+
12+
[dependencies]
13+
bitwarden-error = { workspace = true }
14+
log = { workspace = true }
15+
serde = { workspace = true }
16+
serde_json = { workspace = true }
17+
thiserror = { workspace = true }
18+
tokio = { features = ["sync", "time", "rt"], workspace = true }
19+
20+
[target.'cfg(target_arch="wasm32")'.dependencies]
21+
js-sys = { workspace = true }
22+
tsify-next = { workspace = true }
23+
wasm-bindgen = { workspace = true }
24+
wasm-bindgen-futures = { workspace = true }
25+
26+
[dev-dependencies]
27+
async-trait = "0.1.88"
28+
console_error_panic_hook = "0.1.7"
29+
js-sys = { workspace = true }
30+
tsify-next = { workspace = true }
31+
wasm-bindgen = { workspace = true }
32+
wasm-bindgen-futures = { workspace = true }
33+
wasm-bindgen-test = { workspace = true }
34+
35+
[lints]
36+
workspace = true

crates/bitwarden-threading/README.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
# bitwarden-threading
2+
3+
Utility crate for Bitwarden SDK to handle threading and async quirks in FFI contexts.
4+
5+
## WASM Testing
6+
7+
To run the WASM tests, you can use the following command:
8+
9+
```bash
10+
cargo test --target wasm32-unknown-unknown --all-features -- --nocapture
11+
```

crates/bitwarden-threading/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
mod thread_bound_runner;
2+
3+
pub use thread_bound_runner::ThreadBoundRunner;
Lines changed: 225 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,225 @@
1+
#![allow(dead_code)]
2+
#![allow(unused_variables)]
3+
4+
use std::{future::Future, pin::Pin, rc::Rc};
5+
6+
use bitwarden_error::bitwarden_error;
7+
use thiserror::Error;
8+
#[cfg(not(target_arch = "wasm32"))]
9+
use tokio::task::spawn_local;
10+
#[cfg(target_arch = "wasm32")]
11+
use wasm_bindgen_futures::spawn_local;
12+
13+
type CallFunction<ThreadState> =
14+
Box<dyn FnOnce(Rc<ThreadState>) -> Pin<Box<dyn Future<Output = ()>>> + Send>;
15+
16+
struct CallRequest<ThreadState> {
17+
function: CallFunction<ThreadState>,
18+
}
19+
20+
/// The call failed before it could return a value. This should not happen unless
21+
/// the thread panics, which can only happen if the function passed to `run_in_thread`
22+
/// panics.
23+
#[derive(Debug, Error)]
24+
#[error("The call failed before it could return a value: {0}")]
25+
#[bitwarden_error(basic)]
26+
pub struct CallError(String);
27+
28+
/// A runner that takes a non-`Send`, non-`Sync` state and makes it `Send + Sync` compatible.
29+
///
30+
/// `ThreadBoundRunner` is designed to safely encapsulate a `!Send + !Sync` state object by
31+
/// pinning it to a single thread using `spawn_local`. It provides a `Send + Sync` API that
32+
/// allows other threads to submit tasks (function pointers or closures) that operate on the
33+
/// thread-bound state.
34+
///
35+
/// Tasks are queued via an internal channel and are executed sequentially on the owning thread.
36+
///
37+
/// # Example
38+
/// ```ignore
39+
/// let runner = ThreadBoundRunner::new(my_state);
40+
///
41+
/// runner.run_in_thread(|state| async move {
42+
/// // do something with `state`
43+
/// });
44+
/// ```
45+
///
46+
/// This pattern is useful for interacting with APIs or data structures that must remain
47+
/// on the same thread, such as GUI toolkits, WebAssembly contexts, or other thread-bound
48+
/// environments.
49+
#[derive(Clone)]
50+
pub struct ThreadBoundRunner<ThreadState> {
51+
call_channel_tx: tokio::sync::mpsc::Sender<CallRequest<ThreadState>>,
52+
}
53+
54+
impl<ThreadState> ThreadBoundRunner<ThreadState>
55+
where
56+
ThreadState: 'static,
57+
{
58+
pub fn new(state: ThreadState) -> Self {
59+
let (call_channel_tx, mut call_channel_rx) =
60+
tokio::sync::mpsc::channel::<CallRequest<ThreadState>>(1);
61+
62+
spawn_local(async move {
63+
let state = Rc::new(state);
64+
while let Some(request) = call_channel_rx.recv().await {
65+
spawn_local((request.function)(state.clone()));
66+
}
67+
});
68+
69+
ThreadBoundRunner { call_channel_tx }
70+
}
71+
72+
/// Submit a task to be executed on the thread-bound state.
73+
///
74+
/// The provided function is executed on the thread that owns the internal `ThreadState`,
75+
/// ensuring safe access to `!Send + !Sync` data. Tasks are dispatched in the order they are
76+
/// received, but because they are asynchronous, multiple tasks may be in-flight and running
77+
/// concurrently if their futures yield.
78+
///
79+
/// # Returns
80+
/// A future that resolves to the result of the function once it has been executed.
81+
pub async fn run_in_thread<F, Fut, Output>(&self, function: F) -> Result<Output, CallError>
82+
where
83+
F: FnOnce(Rc<ThreadState>) -> Fut + Send + 'static,
84+
Fut: Future<Output = Output>,
85+
Output: Send + Sync + 'static,
86+
{
87+
let (return_channel_tx, return_channel_rx) = tokio::sync::oneshot::channel();
88+
let request = CallRequest {
89+
function: Box::new(|state| {
90+
Box::pin(async move {
91+
let result = function(state);
92+
return_channel_tx.send(result.await).unwrap_or_else(|_| {
93+
log::warn!(
94+
"ThreadBoundDispatcher failed to send result back to the caller"
95+
);
96+
});
97+
})
98+
}),
99+
};
100+
101+
self.call_channel_tx
102+
.send(request)
103+
.await
104+
.expect("Call channel should not be able to close while anything still still has a reference to this object");
105+
return_channel_rx
106+
.await
107+
.map_err(|e| CallError(e.to_string()))
108+
}
109+
}
110+
111+
#[cfg(test)]
112+
mod test {
113+
use super::*;
114+
115+
/// Utility function to run a test in a local context (allows using tokio::..::spawn_local)
116+
async fn run_test<F>(test: F) -> F::Output
117+
where
118+
F: std::future::Future,
119+
{
120+
#[cfg(not(target_arch = "wasm32"))]
121+
{
122+
let local_set = tokio::task::LocalSet::new();
123+
local_set.run_until(test).await
124+
}
125+
126+
#[cfg(target_arch = "wasm32")]
127+
{
128+
test.await
129+
}
130+
}
131+
132+
async fn run_in_another_thread<F>(test: F)
133+
where
134+
F: std::future::Future + Send + 'static,
135+
F::Output: Send,
136+
{
137+
#[cfg(not(target_arch = "wasm32"))]
138+
{
139+
tokio::spawn(test).await.expect("Thread panicked");
140+
}
141+
142+
#[cfg(target_arch = "wasm32")]
143+
{
144+
test.await;
145+
}
146+
}
147+
148+
#[derive(Default)]
149+
struct State {
150+
/// This is a marker to ensure that the struct is not Send
151+
_un_send_marker: std::marker::PhantomData<*const ()>,
152+
}
153+
154+
impl State {
155+
pub fn add(&self, input: (i32, i32)) -> i32 {
156+
input.0 + input.1
157+
}
158+
159+
#[allow(clippy::unused_async)]
160+
pub async fn async_add(&self, input: (i32, i32)) -> i32 {
161+
input.0 + input.1
162+
}
163+
}
164+
165+
#[tokio::test]
166+
async fn calls_function_and_returns_value() {
167+
run_test(async {
168+
let runner = ThreadBoundRunner::new(State::default());
169+
170+
let result = runner
171+
.run_in_thread(|state| async move {
172+
let input = (1, 2);
173+
state.add(input)
174+
})
175+
.await
176+
.expect("Calling function failed");
177+
178+
assert_eq!(result, 3);
179+
})
180+
.await;
181+
}
182+
183+
#[tokio::test]
184+
async fn calls_async_function_and_returns_value() {
185+
run_test(async {
186+
let runner = ThreadBoundRunner::new(State::default());
187+
188+
let result = runner
189+
.run_in_thread(|state| async move {
190+
let input = (1, 2);
191+
state.async_add(input).await
192+
})
193+
.await
194+
.expect("Calling function failed");
195+
196+
assert_eq!(result, 3);
197+
})
198+
.await;
199+
}
200+
201+
#[tokio::test]
202+
async fn can_continue_running_if_a_call_panics() {
203+
run_test(async {
204+
let runner = ThreadBoundRunner::new(State::default());
205+
206+
runner
207+
.run_in_thread::<_, _, ()>(|state| async move {
208+
panic!("This is a test panic");
209+
})
210+
.await
211+
.expect_err("Calling function should have panicked");
212+
213+
let result = runner
214+
.run_in_thread(|state| async move {
215+
let input = (1, 2);
216+
state.async_add(input).await
217+
})
218+
.await
219+
.expect("Calling function failed");
220+
221+
assert_eq!(result, 3);
222+
})
223+
.await;
224+
}
225+
}

0 commit comments

Comments
 (0)