`single_threaded_task_pool` `scope` api doesn't work in wasm

[mockersf]

Bevy version

0.5 and d119c1ce14da59089a65373d59715a41d05251ad

Operating system & version

Firefox & Chromium

What you did

Trying to use the task pool to execute several tasks, I hoped to have the same code running in both wasm and native

use bevy::{prelude::*, tasks::prelude::*};

fn main() {
    let mut builder = App::build();
    builder.add_plugins(DefaultPlugins);

    let asset_io = bevy::asset::create_platform_default_asset_io(&mut builder);

    builder
        .insert_resource(asset_io)
        .init_resource::<Vec<String>>()
        .add_startup_system(get_from_future.system())
        .add_system(log_stuff.system())
        .run();
}

fn get_from_future(
    task_pool: Res<ComputeTaskPool>,
    mut expensive_to_get: ResMut<Vec<String>>,
    asset_io: Res<Box<dyn bevy::asset::AssetIo>>,
) {
    let asset_io = &*asset_io;
    *expensive_to_get = task_pool
        .scope(|scope| {
            for i in 0..3 {
                scope.spawn(async move {
                    std::str::from_utf8(
                        &asset_io
                            .load_path(&std::path::Path::new(&format!("file_{}.big", i)))
                            .await
                            .unwrap(),
                    )
                    .unwrap()
                    .to_string()
                });
            }
        })
        .into_iter()
        .collect();
}

fn log_stuff(expensive_to_get: Res<Vec<String>>) {
    bevy::log::warn!("got {:?}", *expensive_to_get);
}

This works on native!

Building this for wasm32 gives an error:

26 |                 scope.spawn(async move {
   |                       ^^^^^ future created by async block is not `Send`

Indeed, spawn requires the future to be Send in the single_threaded_task_pool. This could probably be removed anyway as it calls spawn_local which doesn't need Send

bevy/crates/bevy_tasks/src/single_threaded_task_pool.rs

Lines 129 to 131 in d119c1c

	pub fn spawn<Fut: Future<Output = T> + 'scope + Send>(&mut self, f: Fut) {
	self.spawn_local(f);
	}

Let's try to run this with spawn_local to check if it would work better.At least it compiles that way!

And crash in the browser

test.js:276 panicked at 'assertion failed: `(left == right)`
  left: `true`,
 right: `false`: cannot recursively acquire mutex',

And rightly so. In the scope function:

bevy/crates/bevy_tasks/src/single_threaded_task_pool.rs

Lines 79 to 83 in d119c1c

	scope
	.results
	.iter()
	.map(|result| result.lock().unwrap().take().unwrap())
	.collect()

The lock() is on a mutex, and that can't work because you can't block in wasm...

Using the task pool in wasm will work for trivial task that aren't long enough to lock the mutex, but will fail for any real case...

I tried a few things but couldn't get it to work

[TheRawMeatball]

I'm not sure if the mutex is the problematic part here - because of the code surrounding code, these mutexes shouldn't be contested. The only other place these mutexes could be locked is the task they correspond to, and the above loop should ensure all tasks have already been completed. The locking here should never block.

[mockersf]

they are locked to write the result here:

bevy/crates/bevy_tasks/src/single_threaded_task_pool.rs

Line 137 in d119c1c

result.lock().unwrap().replace(f.await);

and locked to read the result here:

bevy/crates/bevy_tasks/src/single_threaded_task_pool.rs

Line 82 in d119c1c

.map(|result| result.lock().unwrap().take().unwrap())

[bjorn3]

result.lock().unwrap().replace(f.await); should probably be changed to let res = f.await; result.lock().unwrap().replace(res);.

[mockersf]

result.lock().unwrap().replace(f.await); should probably be changed to let res = f.await; result.lock().unwrap().replace(res);.

just tried that, doesn't work 😞

[Error] panicked at 'called `Option::unwrap()` on a `None` value', bevy_tasks/src/single_threaded_task_pool.rs:82:57

Stack:

http://127.0.0.1:4000/target/wasm.js:829:28
wasm-stub@[wasm code]
<?>.wasm-function[1838]@[wasm code]
<?>.wasm-function[12952]@[wasm code]
<?>.wasm-function[3528]@[wasm code]
<?>.wasm-function[5184]@[wasm code]
<?>.wasm-function[9360]@[wasm code]
<?>.wasm-function[8971]@[wasm code]
<?>.wasm-function[9287]@[wasm code]
<?>.wasm-function[8483]@[wasm code]
<?>.wasm-function[199]@[wasm code]
<?>.wasm-function[2871]@[wasm code]
<?>.wasm-function[222]@[wasm code]
<?>.wasm-function[2471]@[wasm code]
<?>.wasm-function[9798]@[wasm code]
wasm-stub@[wasm code]
_dyn_core__ops__function__FnMut__A____Output___R_as_wasm_bindgen__closure__WasmClosure___describe__invoke__h4b7b1b37763fbaeb@[native code]
__wbg_adapter_51@http://127.0.0.1:4000/target/wasm.js:250:134
real@http://127.0.0.1:4000/target/wasm.js:199:21
promiseReactionJob@[native code]


	(anonymous function) (wasm.js:841)
	wasm-stub
	<?>.wasm-function[1838]
	<?>.wasm-function[12952]
	<?>.wasm-function[3528]
	<?>.wasm-function[5184]
	<?>.wasm-function[9360]
	<?>.wasm-function[8971]
	<?>.wasm-function[9287]
	<?>.wasm-function[8483]
	<?>.wasm-function[199]
	<?>.wasm-function[2871]
	<?>.wasm-function[222]
	<?>.wasm-function[2471]
	<?>.wasm-function[9798]
	wasm-stub
	_dyn_core__ops__function__FnMut__A____Output___R_as_wasm_bindgen__closure__WasmClosure___describe__invoke__h4b7b1b37763fbaeb
	__wbg_adapter_51 (wasm.js:250:135)
	real (wasm.js:199)
	promiseReactionJob

[TheRawMeatball]

they are locked to write the result here:

bevy/crates/bevy_tasks/src/single_threaded_task_pool.rs

Line 137 in d119c1c

result.lock().unwrap().replace(f.await);

and locked to read the result here:

bevy/crates/bevy_tasks/src/single_threaded_task_pool.rs

Line 82 in d119c1c

.map(|result| result.lock().unwrap().take().unwrap())

Yes, but shouldn't the while executor.try_tick() {} loop mean all tasks have already been finished and there are no outstanding locks on anything?

[mockersf]

no, it just poll each future once: https://docs.rs/async-executor/1.4.1/async_executor/struct.LocalExecutor.html#method.try_tick

[TheRawMeatball]

But it's executed in a while loop, and try_tick only returns false if there are no tasks left to run: https://docs.rs/async-executor/1.4.1/src/async_executor/lib.rs.html#172

[TheRawMeatball]

And, the unwrap that panics in your last code is the second unwrap:

[Error] panicked at 'called Option::unwrap() on a None value', bevy_tasks/src/single_threaded_task_pool.rs:82:57

[mockersf]

But it's executed in a while loop, and try_tick only returns false if there are no tasks left to run: https://docs.rs/async-executor/1.4.1/src/async_executor/lib.rs.html#172

it pops the tasks from the queue, poll it once, and go to the next. it's not waiting for tasks to be finished or putting them back in the queue