Storing !Send data in World is problematic

[maniwani]

Summary

The World would be a better place type if thread-local data was kept out of it.

World owning !Send data forces us to block the main thread for long periods of time, which prevents useful concurrency. We can resolve this by storing !Send data separately and exposing a Send proxy for systems to send work to the main thread.

Key Terms

• main thread: the initial thread of the program; the only one that can run the winit event polling loop

[maniwani]

Impact on bevy_ecs

!Send resources are a maintenance burden. There aren't many, but having them stored inside World has resulted in some questionable patterns. The scheduler, for example, has to run itself as a Send future so systems that need access to !Send resources can claim the local thread.

The &World system param has inconsistent behavior. It doesn't count as a !Send access, which is a nice band-aid for parallelism, but means a completely valid system like this one will spuriously panic. It needs a NonSend<()> param to force it to run on the local thread.

fn flip_a_coin(world: &World) {
   // do stuff
   /* ... */ 
   
   // access thread-local data
   // will panic if running on different thread
   let resource = world.non_send_resource::<X>();
   
   // do other stuff
   /* ... */
}

Multi-world concurrency (e.g. pipelined rendering) is where things get really weird. If two or more worlds come from the same thread, their schedules will compete for that thread. Every system with NonSend<T> or NonSendMut<T>, every exclusive system, and every command in one schedule will block those in the other schedules!

We can spawn a thread for each concurrent world instead, but a new problem appears. If two worlds come from different threads, and each had !Send resources, it'll be impossible to access both of their !Send resources at the same time. A system can only run on one thread, after all.

If pipelined rendering spawns the render world on the same thread as the main world, it basically can't have its own !Send resources. It can't access them! The schedule for the render pipeline runs on a different thread! OTOH if it spawns on a different thread, then it can't access them during Extract.

So all those choices have weird caveats. But if we separate !Send resources from World, all those problems disappear.

• World can legitimately be Send.
• All worlds can access !Send resources (through a proxy) without the worlds themselves having to stay in a particular thread.
• The scheduler won't need to care which threads are available.

---

Speaking of World lying about being Send. There's also a soundness bug right now. You can send a World (with its !Send resources) to another thread and drop it, which is UB.

Even a "quick" fix for that (aside from just panicking) would unsurprisingly involve separating the !Send stuff.

• Since resources have their own storage now, we can further divide it into Resources and NonSendResources. Then we can introduce a SendWorld that doesn't have NonSendResources and use it to mediate ops like Extract.
• World would then become !Send but remain Sync.

pub struct World {
     /* ... */
     resources: Resources,
     non_send_resources: NonSendResources,
     /* ... */
}

impl World {
    pub fn split(self) -> (SendWorld, NonSendResources) { /* ... */ }
    pub fn combine(send: SendWorld, non_send: NonSendResources) -> World  { /* ... */ }
}

pub struct SendWorld {
     /* ... */
     resources: Resources,
     /* ... */
}

impl SendWorld {
    pub fn into_world(self) -> World { /* ... */ }
}

[maniwani]

World gets moved between threads in "pipelined rendering" branches. The render world extracts data from the main world, then we move it into another thread to run the render pipeline.

If we want multi-world apps, we basically need them to be Send. Otherwise, it'll be impossible to write a custom App runner that moves some into dedicated threads. You can't pass non-'static references into a non-scoped thread.

[soqb]

i see.
but why couldn't we create the render world on the render thread and give it mutable access to the main world? are there certain !Send resources in the main world that the render world needs to extract?

[maniwani]

Even if we spawned the render world in the render thread, we can't pass a &'a mut World into another thread unless 'a is 'static.

Beyond that, extraction is written using systems. Right now, we insert the main world as a resource (literally Res<MainWorld>) and resource types have to be T: 'static. So struct Wrapper<'a>(&'a mut World) can't be one.

I don't think the render world extracts from the main world's !Send resources directly, but there is an extract system that has to run on the main thread (prepare_windows).

[soqb]

thanks for this detailed explanation!

[hymm]

Not sure the "quick" fix is worth it. It'll be adding a lot of code duplication for forwarding all the methods. I'd be more on board if we had delegation. Probably better just to move on to a final solution.

Worth mentioning that #6534 is a quick fix to at least close the soundness bug.

[maniwani]

Broader Impact

Outside of bevy_ecs, the biggest problem with storing !Send data inside the World is that app updates block the winit event loop!

The event loop being blocked while we're running the app schedule causes a bunch of problems:

• Input lag with vsync enabled due to limited polling rates #3317
• Variable frametime during user input #4691
• Per-Window Event Loops #5645
• Inputs are not processed correctly if key repeat rate is faster than FPS #5909
• Inputs can be missed (or duplicated) when using a fixed time step #6183

For a quick rundown on where input events come from (in general):

1. Your USB input device sets a flag that its state has changed.
2. Your OS polls your USB input device at a periodic rate (e.g. 1kHz), sees that yours has new info, and downloads it as an HID report.
3. Your OS uses the device driver to translate the HID report into more meaningful state.
4. Your OS then copies new state to some memory location visible to user programs.
5. The "events" come from diffing the new state against the previous state and seeing what changed. Your OS might do this for you and notify user programs automatically (i.e. most platforms have built-in keyboard and mouse events, Linux has evdev for all devices), but otherwise the user program might be able to poll the memory location and generate events manually.

winit events are somewhat special since they come from the OS/browser, so mouse clicks might be delayed, but never missed¹. Still, calling app.update() in the event loop makes it impossible for us to observe (and timestamp²) winit events as they appear. Likewise, processing winit events delays each app update.

However, if !Send resources were stored separately from a World, worlds could just live in other threads permanently, removing these delays and giving us the ability to accurately timestamp all³ input events.

Also, while winit removed poll_events for platform consistency, we can replicate on-demand polling by having the main thread send events to another thread through a channel (and calling try_recv on its receiver).

Footnotes

1. But they might be discarded if not read quickly enough. ↩

2. See the videos here for a clear example of why knowing the exact time of input events is useful. Having a background polling thread is required for this. Godot will have one in its upcoming version. It's the only way to do framerate-independent input, and that really shines in XR apps and rhythm games. ↩

3. gilrs is already capable of this on Windows and macOS where it spawns a dedicated polling / event creation thread. ↩

[maniwani]

!Send in the engine

Just so readers are aware. I'll cover the !Send data we currently have, why they're !Send, and how they're used.

winit::event_loop::EventLoop

We have a target platform that, for reasons, needs a window created before the RenderPlugin (wgpu) is setup, and we did it like this.

bevy/crates/bevy_winit/src/lib.rs

Lines 47 to 59 in 72fbcc7

	let event_loop = EventLoop::new();
	#[cfg(not(any(target_os = "android", target_os = "ios", target_os = "macos")))]
	let mut create_window_reader = WinitCreateWindowReader::default();
	#[cfg(any(target_os = "android", target_os = "ios", target_os = "macos"))]
	let create_window_reader = WinitCreateWindowReader::default();
	// Note that we create a window here "early" because WASM/WebGL requires the window to exist prior to initializing
	// the renderer.
	// And for ios and macos, we should not create window early, all ui related code should be executed inside
	// UIApplicationMain/NSApplicationMain.
	#[cfg(not(any(target_os = "android", target_os = "ios", target_os = "macos")))]
	handle_create_window_events(&mut app.world, &event_loop, &mut create_window_reader.0);
	app.insert_resource(create_window_reader)
	.insert_non_send_resource(event_loop);

[maniwani]

This is only a resource temporarily, before the app even starts.

We can just move the storage of !Send resources to the App. It doesn't have to be the World.

[hymm]

Found one more usage. When the tracing_chrome feature is enabled bevy_log inserts a nonsend guard into the world. When the guard is dropped, the tracing file is written.

https://github.com/bevyengine/bevy/blob/main/crates/bevy_log/src/lib.rs#L149

[hymm]

also DebugAssetApp. But I imagine that can be turned into a SubApp and whatever solution we use there.

[maniwani]

bevy_winit::WinitWindows

This type creates and stores winit::window::Window instances. The winit::window::WindowBuilder requires a reference to the event loop, so creating windows can only happen on the main thread. winit::window::Window is Send + Sync but looking at its docs, half its methods can only be called on the main thread (on certain platforms).

That said, we already shim/proxy most interactions through bevy_window::Window.

bevy/crates/bevy_window/src/window.rs

Lines 298 to 373 in 72fbcc7

	/// A command to be sent to a window.
	///
	/// Bevy apps don't interact with this `enum` directly. Instead, they should use the methods on [`Window`].
	/// This `enum` is meant for authors of windowing plugins. See the documentation on [`crate::WindowPlugin`] for more information.
	#[derive(Debug, PartialEq)]
	#[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))]
	pub enum WindowCommand {
	/// Set the window's [`WindowMode`].
	SetWindowMode {
	mode: WindowMode,
	resolution: UVec2,
	},
	/// Set the window's title.
	SetTitle {
	title: String,
	},
	/// Set the window's scale factor.
	SetScaleFactor {
	scale_factor: f64,
	},
	/// Set the window's resolution.
	SetResolution {
	logical_resolution: Vec2,
	scale_factor: f64,
	},
	/// Set the window's [`PresentMode`].
	SetPresentMode {
	present_mode: PresentMode,
	},
	/// Set whether or not the window is resizable.
	SetResizable {
	resizable: bool,
	},
	/// Set whether or not the window has decorations.
	///
	/// Examples of decorations include the close, full screen, and minimize buttons
	SetDecorations {
	decorations: bool,
	},
	/// Set whether or not the cursor's position is locked.
	SetCursorGrabMode {
	grab_mode: CursorGrabMode,
	},
	/// Set the cursor's [`CursorIcon`].
	SetCursorIcon {
	icon: CursorIcon,
	},
	/// Set whether or not the cursor is visible.
	SetCursorVisibility {
	visible: bool,
	},
	/// Set the cursor's position.
	SetCursorPosition {
	position: Vec2,
	},
	/// Set whether or not the window is maximized.
	SetMaximized {
	maximized: bool,
	},
	/// Set whether or not the window is minimized.
	SetMinimized {
	minimized: bool,
	},
	/// Set the window's position on the selected monitor.
	SetPosition {
	monitor_selection: MonitorSelection,
	position: IVec2,
	},
	/// Sets the position of the window to be in the center of the selected monitor.
	Center(MonitorSelection),
	/// Set the window's [`WindowResizeConstraints`]
	SetResizeConstraints {
	resize_constraints: WindowResizeConstraints,
	},
	Close,
	}

prepare_windows in the RenderPlugin creates wgpu::Surfaces using raw window handles, which can only be done on the main thread.

bevy/crates/bevy_render/src/view/window.rs

Lines 187 to 189 in 72fbcc7

	// NOTE: On some OSes this MUST be called from the main thread.
	let surface = render_instance
	.create_surface(&window.raw_handle.as_ref().unwrap().get_handle());

[maniwani]

I think the only extra thing we'd need to decouple app updates from the main thread is a channel to receive events from the event loop.

#[derive(Resource)]
struct Winit {
    recv: Receiver<WinitEvent>,
}

That can have our poll_events method.

[maniwani]

gilrs::Gilrs (Linux only)

On Linux, Gilrs creates a udev context in the calling thread. Windows and macOS both spawn a dedicated polling thread.

The man pages for libudev has language suggesting that all its stuff is !Send + !Sync (although not limited to the main thread).

All functions listed here are thread-agnostic and only a single specific thread may operate on a given object during its entire lifetime. It's safe to allocate multiple independent objects and use each from a specific thread in parallel. However, it's not safe to allocate such an object in one thread, and operate or free it from any other, even if locking is used to ensure these threads don't operate on it at the very same time.

[maniwani]

For this one, I think we can just make a Linux cfg! in bevy_gilrs. The alternative is we upstream a change to gilrs to have its Linux impl spawn a polling thread and make it own the !Send stuff.

[maniwani]

bevy_audio::AudioOutput

Idk much about this one, but it should probably live in the audio thread. Other audio plugins like bevy_oddio seem to work just fine without using any !Send resources.

[harudagondi]

• Related PR: [Merged by Bors] - Make AudioOutput a Resource #6436

The problem is that bevy_audio requires an OutputStream to be alive during the remainder of the program, however storing this type in a struct will make that struct !Send and !Sync. According the comments in cpal:

// The following zero-sized types are for applying Send/Sync restrictions to ensure
// consistent behaviour across different platforms. These verbosely named types are used
// (rather than using the markers directly) in the hope of making the compile errors
// slightly more helpful.
//
// TODO: Remove these in favour of using negative trait bounds if they stabilise.

// A marker used to remove the `Send` and `Sync` traits.
struct NotSendSyncAcrossAllPlatforms(std::marker::PhantomData<*mut ()>);

PR #6436 aims to solve this by leaking OutputStream. By using std::mem::forget, the destructor for OutputStream will not run, and audio playback will not stop. This is fine when you are using AudioSources exclusively, as the AudioOutput will only be instantiated once for AudioOutput<Source = AudioSource>, however this will increase for every Source type added. I have noted this in the abovementioned PR.

[maniwani]

(reserved in case I missed any)

[maniwani]

!Send (or Send + !Sync) in third-party plugins

Please comment here if you know of or maintain a plugin that uses !Send resources (edit: or have Send + !Sync that are treated as such).

[harudagondi]

bevy_kira_audio has a AudioOutput type which is initialized as a non-send resource. This is because the aforementioned type stores a kira::manager::AudioManager, which is a Send and !Sync type.

cc @NiklasEi

[maniwani]

So kira::manager::AudioManager is Send + !Sync but it's "miscast" as !Send because bevy's access model does not have a thing for !Sync?

I think winit::event_loop::EventLoopProxy is the same way.

!Sync should be a small enough extension to the access model. Just need a NonSync<R: Send>. Alternatively, I think @JoJoJet was working on making it part the #[derive(Resource)] macro, then we could just do R::is_sync() at runtime.

[JoJoJet]

#6572 is how I'd support !Sync resources.

[Bluefinger]

My RNG types for turborand are Send + !Sync as they use Cell/UnsafeCell internally. As such, when exposing them in bevy_turborand, I have to use the &mut ref trick to be able to expose the Rng methods safely (making them Sync effectively) for Component or Resource. The current situation does require a bit of untidy workaround and I think is a blocker for enabling reflection for my Rng types, example code in bevy_turborand

[zicklag]

A !Sync + !Send resource is used to store the the JS scripting engine in bevy_mod_js_scripting. I don't think it'd be too hard to move to it's own thread and then send it jobs over a channel. Although it needs to be able to borrow the Bevy world, so the Bevy world would have to be Sync, I think.

[maniwani]

Thread-Local Executor

This is my main proposal for decoupling !Send storage from World.

App can store !Send resources during construction. When we start the app, we can unpack it into its individual parts (sub-app collection and thread-local resources) and just leave the !Send stuff on the main thread.

On most targets, we can then move the world(s) off the main thread upfront and proxy any "must run on the ___ thread" operations through an executor. Rather than force the entire system onto the main thread, we can just scope the parts that borrow !Send data.

A system that looks like this:

// This system can only run on the main thread.
fn foo(mut x: ResMut<X>, y: NonSend<Y>) {
    *x.field = y.func();
}

Would be rewritten like this:

// This system can run on any thread.
fn foo(mut x: ResMut<X>, main_thread: Res<ThreadLocal<Main>>) {
    // Only this closure runs on the main thread.
    *x.field = main_thread.run(|tls: &mut NonSendResources| {
        let y = tls.get::<Y>();
        y.func()
    });
}

Or, if we preserve more of the existing ergonomics by adding a NonSendParam (bikeshed name) family of traits, like this:

// This system can run on any thread.
fn foo(mut x: ResMut<X>, main_thread: Res<ThreadLocal<Main>>) {
    // Only this closure runs on the main thread.
    *x.field = main_thread.run(|y: &Y| y.func());
}

TODO: Show example app runner.

Change Detection

If !Send resources don't belong to any world, then they clearly can't use the change ticks of one. The options are:

• !Send container has its own change counter, and ThreadLocal<Marker> (full-fledged system param) accesses it.
• Everything in an App shares one change counter and every world gets an Arc<AtomicU*> to it.
• !Send resources don't have change detection.

[hymm]

how can you access multiple non send resources at the same time in this model?

[maniwani]

how can you access multiple non send resources at the same time in this model?

We can make the get on NonSendResources accept any P: NonSendParam tuple.

So like this:

fn foo(main_thread: Res<ThreadLocal<Main>>) {
    main_thread.run(|tls: &mut NonSendResources| {
        let (a, mut b, mut c) = tls.get::<(&A, &mut B, &mut C)>();
        /* ... */
    });
}

Or like this:

fn foo(main_thread: Res<ThreadLocal<Main>>) {
    main_thread.run(|a: &A, b: &mut B, c: &mut C| {
         /* ... */
    });
}

NonSendResources would be !Send + !Sync, so if we have the param trait, it'd look something like this:

pub trait NonSendParamFetch<'tls> {
    type Item: NonSendParam<Fetch = Self>;
    
    unsafe fn get_param(tls: &'tls UnsafeCell<NonSendResources>, /* other args... */) -> Self::Item;
}

[maniwani]

Bring your own event loop

If you're writing a plugin that spawns its own dedicated thread, then unless something is specifically tied to the main thread, you can avoid !Send resources. Dedicated threads basically run event loops, and if winit, gilrs, and audio libraries can provide thread-safe interfaces for their event loops, you can too. The author of the audio library kira wrote a great blog post on how to do this.

The suggested ThreadLocalExecutor can be pub if users and plugin authors want to build or prototype with it.

[maniwani]

TODO: cfg! for single-threaded targets like wasm32

[maniwani]

Send + !Sync

Options:

1. Keep Resource: Send + Sync and have users wrap any Send + !Sync types in a SyncCell (Exclusive) to use them as resources.
2. Extend scheduler/Access model to distinguish !Sync (direct access, e.g. World::get_resource::<R>(), would still be unsound).

[hymm]

Use a thread local abstraction and change NonSend to be a marker to run on main thread

We can use a lib like https://docs.rs/thread-local-object/latest/thread_local_object/ to keep storing the NonSend reources in World. Then we just make NonSend something that tells the system to run on the main thread.

fn my_system(non_send_res: NonSend<Res<ThreadLocal<MyResource>>>) {
    non_send_res.get(| resource | {
        // do something with the resource.
    });
}

// this could potentially work with components too
fn my_query_system(non_send_q: NonSend<Query<ThreadLocal<MyComponent>>> {
    for my_component in &non_send_q {
        my_component.get(| my_c | {
            // do something with the component
        });
    }
}

There are at least a couple disadvantages to this approach. The lib above doesn't drop the contained value when the ThreadLocal is dropped. Only when the thread is exited. Also this keeps the complexity of scheduling resources in the scheduler rather than moving it out of it like the above recommended method.

The advantages are that it keeps the values "owned" by the world and can even work for NonSend Components.

edit: Another disadvatage is that the lib I'm using only allows for usize number of ThreadLocal instances ever existing. This is especially a problem because it doesn't reuse it's internal index. i.e. dropping a resource would not free it's index. This could probably be worked around by rolling our own solution though.

[maniwani]

Also, as long as !Send data is in the World, we're needlessly blocking the winit event loop, because the schedule's entry point would still be the main thread. (edit: not relevant)

[hymm]

nothing in this proposal prevents moving the world off the main thread. Things that need the main thread would just need to be initialized before the world gets sent.

[maniwani]

We can use a lib like docs.rs/thread-local-object/latest/thread_local_object to keep storing the NonSend resources in World.

I got the wrong impression then. thread_local_object::ThreadLocal<T> does not actually store the !Send data in the World.

So the unique pros of each are:

• thread executor
  • only blocks main thread inside the closure blocks
  • less scheduler complexity
  • does not use thread_local!
• thread_local_object
  • the world still gives off the appearance of owning the resources
  • could do !Send components

[hymm]

made a POC of this #6657 as it was easy to adapt from my earlier PR. Not sure which approach I prefer yet. Need to think about it more.

[hymm]

I did some preliminary work on the thread_executor approach and I'd say another downside is having to duplicate a lot of the apis on world to allow them to take nonsend resources. This is close to the NonSend/Send World approach.

[SkiFire13]

I was looking at the thread-local-object proposal and was wondering why it doesn't use something like NonSend/Fragile?

Pros:

• no weird closure API
• non-send resources would be stored in the World
• doesn't have the usize::MAX cap
• it should be more efficient: no need to lookup a thread-local HashMap at every access
• fragile doesn't seem to have a security advisory unlike thread-local-object
• has the same behaviour as [Merged by Bors] - Panic on dropping NonSend in non-origin thread. #6534 but doesn't need to split Resources and NonSendResources

Cons:

• panics on dropping these kind of resources on non-origin thread
• allows inserting these kind of resources from every thread, e.g. from Commands. This is both more flexible than the current approach but also more dangerous, since it can introduce non-determinism when such resources are created on some threads and used on others (although nothing prevents users from using Fragile right now and doing the same thing)

Possible extensions include:

• add a method on the Resource to check whether the Resource can be used on the main thread only. This could be automatically implemented for Fragile<T> (or more likely some wrapper around it)
  • This can the be used a sanity check when inserting resources, like the current World::validate_non_send_access.
  • It could also be used to mark threads that should run on the main thread
• a similar approach could be extended to !Send + Sync components if needed
• maybe a similar approach could be extended to Components?

[hymm]

There are soundness issues in that crate that I would be ok as a user dealing with, but would strongly hesitate to put in another library. And the soundness issues are likely unfixable at the library level as std had issues with a similar api that they eventually moved to the current closure approach.

[SkiFire13]

AFAIK the soundness issues are restricted only to the fragile::Sticky type, which uses an approach like thread-local-object (store the value in a thread local and drop it when the thread ends). fragile::Fragile instead doesn't use thread local storage, so it doesn't suffer from those issues.