Skip to content

Commit 8591afa

Browse files
authored
Fix stray &mut self in Wasm (#65)
1 parent 534bbbc commit 8591afa

File tree

10 files changed

+161
-21
lines changed

10 files changed

+161
-21
lines changed

.github/workflows/quality.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,3 +37,6 @@ jobs:
3737

3838
- name: Clippy
3939
run: cargo clippy --workspace --all-targets --all-features -- -D warnings
40+
41+
- name: Lint wasm_bindgen &mut boundaries
42+
run: ./scripts/lint-wasm-mut.sh --github-annotations

Cargo.lock

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

automerge_sedimentree_wasm/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "automerge_sedimentree_wasm"
3-
version = "0.4.1"
3+
version = "0.4.2"
44
description = "Wasm bindings for storing Automerge documents in Sedimentree"
55
categories = ["web-programming"]
66
keywords = ["automerge", "sedimentree", "wasm"]

automerge_sedimentree_wasm/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@automerge/automerge-sedimentree",
3-
"version": "0.4.1-alpha.0",
3+
"version": "0.4.2-alpha.0",
44
"description": "Wasm bindings for storing Automerge documents in Sedimentree",
55
"publishConfig": {
66
"access": "public"

automerge_sedimentree_wasm/src/fragment.rs

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
//! Fragment-related wrappers.
22
33
use alloc::vec::Vec;
4+
use core::cell::RefCell;
45
use sedimentree_core::collections::{Map, Set};
56

67
use sedimentree_core::{commit::FragmentState, crypto::digest::Digest, loose_commit::LooseCommit};
@@ -117,10 +118,15 @@ impl From<Map<WasmDigest, Set<WasmDigest>>> for WasmBoundary {
117118
}
118119

119120
/// A store for fragment states.
121+
///
122+
/// Uses interior mutability (`RefCell`) so that `wasm_bindgen` only takes
123+
/// a shared borrow across the JS boundary, avoiding "recursive use of
124+
/// an object" panics when JS callbacks re-enter during `buildFragmentStore`.
120125
#[derive(Debug, Default, Clone)]
121126
#[wasm_bindgen(js_name = FragmentStateStore)]
122127
pub struct WasmFragmentStateStore(
123-
pub(crate) Map<Digest<LooseCommit>, FragmentState<Set<Digest<LooseCommit>>>>,
128+
#[allow(clippy::type_complexity)]
129+
pub(crate) RefCell<Map<Digest<LooseCommit>, FragmentState<Set<Digest<LooseCommit>>>>>,
124130
);
125131

126132
#[wasm_bindgen(js_class = FragmentStateStore)]
@@ -129,17 +135,20 @@ impl WasmFragmentStateStore {
129135
#[wasm_bindgen(constructor)]
130136
#[must_use]
131137
pub fn new() -> Self {
132-
Self(Map::new())
138+
Self(RefCell::new(Map::new()))
133139
}
134140

135141
/// Insert a fragment state into the store.
136-
pub fn insert(&mut self, digest: &WasmDigest, state: &WasmFragmentState) {
137-
self.0.insert(digest.clone().into(), state.0.clone());
142+
pub fn insert(&self, digest: &WasmDigest, state: &WasmFragmentState) {
143+
self.0
144+
.borrow_mut()
145+
.insert(digest.clone().into(), state.0.clone());
138146
}
139147

140148
/// Get a fragment state from the store.
141149
pub fn get(&self, digest: &WasmDigest) -> Option<WasmFragmentState> {
142150
self.0
151+
.borrow()
143152
.get(&digest.clone().into())
144153
.cloned()
145154
.map(WasmFragmentState)
@@ -151,6 +160,6 @@ impl From<WasmFragmentStateStore>
151160
for Map<Digest<LooseCommit>, FragmentState<Set<Digest<LooseCommit>>>>
152161
{
153162
fn from(store: WasmFragmentStateStore) -> Self {
154-
store.0
163+
store.0.into_inner()
155164
}
156165
}

automerge_sedimentree_wasm/src/lib.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ impl WasmSedimentreeAutomerge {
5555
hash_metric: &WasmHashMetric,
5656
) -> Result<WasmFragmentState, WasmFragmentError> {
5757
Ok(self
58-
.fragment(head.clone().into(), &known_states.0, hash_metric)
58+
.fragment(head.clone().into(), &known_states.0.borrow(), hash_metric)
5959
.map(WasmFragmentState)?)
6060
}
6161

@@ -69,7 +69,7 @@ impl WasmSedimentreeAutomerge {
6969
pub fn js_build_fragment_store(
7070
&self,
7171
head_digests: Vec<JsDigest>,
72-
known_fragment_states: &mut WasmFragmentStateStore,
72+
known_fragment_states: &WasmFragmentStateStore,
7373
strategy: &WasmHashMetric,
7474
) -> Result<Vec<WasmFragmentState>, WasmFragmentError> {
7575
let heads: Vec<Digest<LooseCommit>> = head_digests
@@ -78,7 +78,7 @@ impl WasmSedimentreeAutomerge {
7878
.collect();
7979

8080
let fresh = self
81-
.build_fragment_store(&heads, &mut known_fragment_states.0, strategy)?
81+
.build_fragment_store(&heads, &mut known_fragment_states.0.borrow_mut(), strategy)?
8282
.into_iter()
8383
.cloned()
8484
.map(WasmFragmentState)

automerge_subduction_wasm/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "automerge_subduction_wasm"
3-
version = "0.2.1"
3+
version = "0.2.2"
44
description = "Wasm bindings for syncing Automerge documents via Subduction"
55
categories = ["web-programming"]
66
keywords = ["automerge", "sedimentree", "wasm"]

automerge_subduction_wasm/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@automerge/automerge-subduction",
3-
"version": "0.2.1-alpha.0",
3+
"version": "0.2.2-alpha.0",
44
"description": "Wasm bindings for syncing Automerge documents via Subduction",
55
"publishConfig": {
66
"access": "public"

nix/commands.nix

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -565,6 +565,14 @@
565565
'';
566566
};
567567

568+
lint = {
569+
# Detect `&mut self` or `&mut T` parameters on #[wasm_bindgen] boundaries.
570+
# These cause "recursive use of an object" runtime panics when JS re-enters
571+
# during the call. Use RefCell for interior mutability instead.
572+
"lint:wasm-mut" = cmd "Lint for &mut on wasm_bindgen boundaries"
573+
''${pkgs.bash}/bin/bash "$WORKSPACE_ROOT/scripts/lint-wasm-mut.sh" --workspace-root "$WORKSPACE_ROOT"'';
574+
};
575+
568576
ci = {
569577
"ci" = cmd "Run full CI suite (build, lint, test, wasm)" ''
570578
set -e
@@ -574,32 +582,36 @@
574582
echo "========================================"
575583
echo ""
576584
577-
echo "===> [1/6] Checking formatting..."
585+
echo "===> [1/7] Checking formatting..."
578586
${cargo} fmt --check
579587
echo "✓ Formatting OK"
580588
echo ""
581589
582-
echo "===> [2/6] Running Clippy..."
590+
echo "===> [2/7] Running Clippy..."
583591
${cargo} clippy --workspace --all-targets -- -D warnings
584592
echo "✓ Clippy OK"
585593
echo ""
586594
587-
echo "===> [3/6] Building host target..."
595+
echo "===> [3/7] Checking for &mut on wasm_bindgen boundaries..."
596+
lint:wasm-mut
597+
echo ""
598+
599+
echo "===> [4/7] Building host target..."
588600
${cargo} build --workspace
589601
echo "✓ Host build OK"
590602
echo ""
591603
592-
echo "===> [4/6] Running host tests..."
604+
echo "===> [5/7] Running host tests..."
593605
${cargo} test --workspace
594606
echo "✓ Host tests OK"
595607
echo ""
596608
597-
echo "===> [5/6] Building wasm packages..."
609+
echo "===> [6/7] Building wasm packages..."
598610
${wasm-pack} build --target web subduction_wasm
599611
echo "✓ Wasm build OK"
600612
echo ""
601613
602-
echo "===> [6/6] Running wasm tests..."
614+
echo "===> [7/7] Running wasm tests..."
603615
${wasm-pack} test --node subduction_wasm
604616
echo "✓ Wasm tests OK"
605617
echo ""
@@ -761,4 +773,4 @@
761773
'';
762774
};
763775
in
764-
bench // bodge // build // ci // fmt // monitoring // release // test // wasm
776+
bench // bodge // build // ci // fmt // lint // monitoring // release // test // wasm

scripts/lint-wasm-mut.sh

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
#!/usr/bin/env bash
2+
set -euo pipefail
3+
4+
# Detect `&mut self` or `&mut T` parameters on #[wasm_bindgen] boundaries.
5+
# These cause "recursive use of an object" runtime panics when JS callbacks
6+
# re-enter during the call. Use RefCell for interior mutability instead.
7+
#
8+
# Usage:
9+
# scripts/lint-wasm-mut.sh [--workspace-root DIR] [--github-annotations]
10+
#
11+
# Options:
12+
# --workspace-root DIR Root of the workspace (default: script's parent dir)
13+
# --github-annotations Format errors as GitHub Actions annotations
14+
15+
workspace_root=""
16+
github_annotations=false
17+
18+
while [[ $# -gt 0 ]]; do
19+
case "$1" in
20+
--workspace-root) workspace_root="$2"; shift 2 ;;
21+
--github-annotations) github_annotations=true; shift ;;
22+
*) echo "Unknown option: $1" >&2; exit 1 ;;
23+
esac
24+
done
25+
26+
if [[ -z "$workspace_root" ]]; then
27+
workspace_root="$(cd "$(dirname "$0")/.." && pwd)"
28+
fi
29+
30+
# Prefer gawk, fall back to awk (works on macOS and ubuntu-latest)
31+
if command -v gawk &>/dev/null; then
32+
AWK=gawk
33+
else
34+
AWK=awk
35+
fi
36+
37+
rc=0
38+
39+
for crate_dir in "$workspace_root"/*_wasm; do
40+
[[ -d "$crate_dir/src" ]] || continue
41+
crate_name="$(basename "$crate_dir")"
42+
43+
# State machine in awk:
44+
# - On `#[wasm_bindgen` attribute -> mark next fn as wasm-exported
45+
# - On `impl` block preceded by `#[wasm_bindgen` -> all fns in the block
46+
# are wasm-exported until brace depth returns to 0
47+
# - Inside a wasm-exported context, flag any `&mut self` or `&mut <Ident>`
48+
# in fn parameter lists
49+
#
50+
# Intentionally conservative: may false-positive on private helpers inside
51+
# a wasm_bindgen impl block. That's fine — the fix (RefCell) is always safe,
52+
# and false negatives (missing a real problem) are worse.
53+
while IFS= read -r src_file; do
54+
$AWK -v github="$github_annotations" '
55+
BEGIN { in_wb_impl = 0; brace_depth = 0; wb_attr = 0; found = 0 }
56+
57+
/^[[:space:]]*#\[wasm_bindgen/ { wb_attr = 1 }
58+
59+
/^[[:space:]]*(pub[[:space:]]+)?impl[[:space:]]/ {
60+
if (wb_attr) { in_wb_impl = 1; brace_depth = 0 }
61+
wb_attr = 0
62+
}
63+
64+
in_wb_impl {
65+
n = split($0, chars, "")
66+
for (i = 1; i <= n; i++) {
67+
if (chars[i] == "{") brace_depth++
68+
if (chars[i] == "}") {
69+
brace_depth--
70+
if (brace_depth <= 0) { in_wb_impl = 0; brace_depth = 0 }
71+
}
72+
}
73+
}
74+
75+
(in_wb_impl || wb_attr) && /fn[[:space:]]+[a-zA-Z_]/ {
76+
if (/&mut[[:space:]]/) {
77+
if (github == "true")
78+
printf "::error file=%s,line=%d::%s\n", FILENAME, NR, $0
79+
else
80+
printf " %s:%d: %s\n", FILENAME, NR, $0
81+
found = 1
82+
}
83+
}
84+
85+
wb_attr && /^[[:space:]]*(pub[[:space:]]+)?fn[[:space:]]/ {
86+
if (/&mut[[:space:]]/) {
87+
if (github == "true")
88+
printf "::error file=%s,line=%d::%s\n", FILENAME, NR, $0
89+
else
90+
printf " %s:%d: %s\n", FILENAME, NR, $0
91+
found = 1
92+
}
93+
wb_attr = 0
94+
}
95+
96+
!/^[[:space:]]*#/ && !/^[[:space:]]*$/ && !/^[[:space:]]*(pub[[:space:]]+)?impl/ && !/^[[:space:]]*(pub[[:space:]]+)?fn/ {
97+
wb_attr = 0
98+
}
99+
100+
END { exit found }
101+
' "$src_file" || {
102+
echo "FAIL: found &mut on wasm_bindgen boundary in $crate_name"
103+
rc=1
104+
}
105+
done < <(find "$crate_dir/src" -name '*.rs' -type f)
106+
done
107+
108+
if [[ "$rc" -eq 0 ]]; then
109+
echo "✓ No &mut on wasm_bindgen boundaries"
110+
else
111+
echo ""
112+
echo "Fix: use RefCell for interior mutability so wasm_bindgen only"
113+
echo "takes shared borrows (&self). See automerge_sedimentree_wasm/"
114+
echo "src/fragment.rs for an example."
115+
exit 1
116+
fi

0 commit comments

Comments
 (0)