Handle subsecond updates asynchronously in Dioxus#4905
Conversation
tekacs
force-pushed
the
async-subsecond-handler
branch
4 times, most recently
from
c59446f to
3eb51fe
Compare
Try to resolve DioxusLabs#4902. Empty merge commit
tekacs
force-pushed
the
async-subsecond-handler
branch
from
3eb51fe to
da87dd3
Compare
|
I closed the other PR since we definitely don't want to run all handlers async, but you said this is the branch you're using to dev against. I don't see how this change would fix an issue where we're borrowing the dom twice, unless an event is actively being handled while we process the hotpatch. That being said, I do see this issue pop up with use_resource so there might be an issue with the async polling and setting the runtime. Do you have a more concrete repro I could work against to see what's actually happening here? |
|
Hullo! I haven't seen an instance of this error pop up since I put this patch on the branch I use for dev. It's been a moment since I opened this PR, but if I recall correctly, the gist is that hot patching is somehow happening while this RefCell in wasm-bindgen-futures is borrowed. You can see that in the panic on the first line:
In my setup I'm likely spawning a lot more futures than a base Dioxus app, because I have various future loops (akin to setTimeout), which run forever. So it's almost a given that every few hot patches it'll panic like this. As for the other PR – I would guess that any uses of Subsecond on WASM using futures would run into this issue, so I'm not certain that a Dioxus-specific fix is the way? I can't pause to produce a repro right this second, but I suspect that this should show up very rapidly if trying to hot patch an app that creates a simple future loop with gloo's timers (really just setTimeout) or something akin. I'll try to supply a proper repro if I get a spare moment though! |
|
I've created a repro for this but I suspect the underlying problem is that wasm-bindgen-futures doesn't guard against re-entrant calls to its task run function from the browser's micro task scheduler so am working on building a reproducer for that, not using Dioxus at all. |
|
I've created a small reproducer - but I can't attest to it being minimal. I went down a rabbit hole or two trying to create a repro that was pure wasm-bindgen-futures and wasn't successful but I think only because I hadn't figured out all that Dioxus is doing to support hotpatch. But I will say learning about the browser's ability to indirectly call into another WASM module was worth it - that is a very cool technology that has been there a long time but I hadn't heard about it before. The Dioxus app is small but there are a number of things it is doing and I've run out of time so don't know if they are all necessary. With the code in place: Below are the app files, and a hammer script. Even with these, the web page on my M2 Macbook Safari didn't hit the RefCell borrow panic unless I moved to another virtual desktop for a moment, clicked on other terminals, and then moved back to the desktop with the page on it; and sometimes a few times, back and forth between virtual desktops, before the race was triggered. I have to assume the browser's microtask scheduler is woken up differently in that scenario, letting the race proceed - and that without letting the web page sleep, the browser's microtask scheduler only kicks in when the event loop loops around normally. I've read that the JS spec gives some latitude as to when the microtask scheduler is woken - different browsers may not all do the same thing. Will also say this is very easy to hit with Safari in my setup but so far I haven't hit it using the Chrome browser. Take that for what it's worth. *RefCell already borrowed* stackCargo.toml[package]
name = "subsecond_refcell_repro"
version = "0.1.0"
edition = "2021"
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
[dependencies]
dioxus = { version = "0.7.1", features = [] }
gloo-timers = { version = "0.3", features = ["futures"] }
js-sys = "0.3"
wasm-bindgen = "0.2"
wasm-bindgen-futures = "0.4"
[features]
default = ["web"]
web = ["dioxus/web"]
desktop = ["dioxus/desktop"]
mobile = ["dioxus/mobile"]
[profile.release]
debug = 0Dioxus.toml[application]
[web.app]
# HTML title tag content
title = "subsecond_refcell_repro"
# include `assets` in web platform
[web.resource]
# Additional CSS style files
style = []
# Additional JavaScript files
script = []
[web.resource.dev]
# Javascript code file
# serve: [dev-server] only
script = []
[web.wasm_opt]
debug = falsesrc/main.rs#![allow(non_snake_case)]
use dioxus::prelude::*;
use gloo_timers::future::TimeoutFuture;
use wasm_bindgen::JsValue;
use wasm_bindgen_futures::JsFuture;
use wasm_bindgen_futures::spawn_local;
// The hammer script toggles this between "A" and "B"
const FLIP: &str = "A";
// Didn't happen on my M2 mac at 200, but one time at 400.
const WORKERS: usize = 1600;
fn label() -> &'static str {
FLIP
}
#[component]
fn Worker(id: usize, gen: Signal<u64>) -> Element {
// This resource is dependency-tracked on gen(), so every gen tick
// cancels and restarts it (lots of churn).
let _r = use_resource(move || {
let v = gen();
async move {
TimeoutFuture::new(0).await;
v
}
});
rsx! { div { "worker {id}" } }
}
fn App() -> Element {
let gen = use_signal(|| 0u64);
// Track flip changes (hotpatch) and burst microtasks right after a flip is observed.
// This increases the odds the patch handler overlaps with wasm-bindgen-futures queue activity.
let mut last_flip = use_signal(|| "");
use_effect(move || {
let now = label();
if last_flip() != now {
last_flip.set(now);
// Burst size knobs: start high; reduce if Safari becomes unusable.
const BURST: usize = 10_000;
spawn_local(async move {
for _ in 0..BURST {
//yield_microtask().await;
// Spawn many recursive chains simultaneously
spawn_recursive_churn(10);
}
});
}
});
// Drive gen forward as fast as setTimeout(0) will allow.
use_future(move || {
let mut gen = gen;
async move {
loop {
gen.set(gen() + 1);
TimeoutFuture::new(0).await;
}
}
});
rsx! {
div {
h1 { "flip: {label()}" }
div { "gen: {gen()}" }
div {
for id in 0..WORKERS {
Worker { key: "{id}", id, gen }
}
}
}
}
}
fn main() {
launch(App);
}
fn spawn_recursive_churn(depth: usize) {
if depth == 0 { return; }
spawn_local(async move {
// This spawn happens DURING the outer spawn's poll
spawn_local(async move {
spawn_recursive_churn(depth - 1);
});
// Keep this future alive longer
for _ in 0..100 {
yield_microtask().await;
}
});
}
async fn yield_microtask() {
// Promise.resolve().then(...) -> microtask turn
let p = js_sys::Promise::resolve(&JsValue::UNDEFINED);
let _ = JsFuture::from(p).await;
}The script that triggers the --hotpatch very quickly. Be warned that each hammer.sh#!/usr/bin/env bash
#
# This hammer.sh is run in a separate terminal
# so we have `dx serve --hotpatch` in one terminal
# and `./hammer.sh` in another.
set -e
set -f
FILE="src/main.rs"
while true; do
perl -0777 -pe 's/const FLIP: &str = "A";/const FLIP: &str = "B";/g' -i "$FILE"
sleep 0.05
perl -0777 -pe 's/const FLIP: &str = "B";/const FLIP: &str = "A";/g' -i "$FILE"
sleep 0.05
done |
|
I think I tracked down the bug recently and fixed it - there was a race condition in apply_patch on wasm where we'd be waiting for the new wasm to compile while the current memory space had been adjusted. If you see the issue crop up again, please LMK because we might need to make apply_patch async with locks to fix it. |
3 participants
Try to resolve #4902.