HalfEdgeMesh cannot be shared between threads safely.
DamenH opened this issue · 1 comments
This makes it difficult to use the HalfEdgeMesh
in a Bevy component, for example.
Adding the sync
feature flag to blackjack should fix this.
Hi! Thanks for reporting 😄 As we discussed on discord, I'm aware of the issue and unfortunately, there are no easy workaround that can be implemented externally in a completely safe way. A HalfEdgeMesh
is both !Sync
and !Send
, the latter occurs because there is an API which the lua bindings use to get a reference to a "shared" channel. That shared channel is backed by an Rc, so obtaining a shared channel and then sending the HalfEdgeMesh
to another thread while that shared channel is still held alive somewhere would cause trouble. This is why common workarounds like wrapping HalfEdgeMesh
in a Mutex
don't work, because mutex is only Send
if its contents are.
To summarize the task: I'd rather this be a "don't pay for what you don't use" thing, so unfortunately we can't simply go through the source and replace all the RefCell
s and Rc
s with their thread-safe counterparts. That would make the library less performant when you don't actually need to share HalfEdgeMesh
between threads (which is in most cases).
The best way to approach this, would be to create wrappers for an "interior mutability" type and a "shared ownership" type. The implementation would simply delegate to RefCell
/ RwLock
and Rc
/ Arc
using conditional compilation.
The wrapper types are necessary because the different types don't offer a shared API, so we need the wrapper type to know when to call lock()
or borrow()
depending on a compile-time feature flag and offer a clean unified API to the rest of the codebase. The wrapper type it would be easier to swap the internal implementation with something else (e.g. replace the RwLock with an AtomicRefCell).
This is not difficult work, but it can be a bit tedious. I'm currenlty focused on the GUI rewrite, so I can't tackle this right now. PRs are welcome as we discussed 😄
But in the meantime, maybe I can suggest a different architecture, similar to what I did for the godot-rust plugin. Instead of storing HalfEdgeMesh
as bevy components, create a thin component type containing an id, something that is cheaply Copy
-able like a slotmap
key. Then, the meshes are actually managed inside a bevy non-send resource. This is similar to how bevy already handles assets, where the asset key is stored inside components, and to access the actual asset data, your system needs to depend on an additional resource. This definitely makes the architecture a bit more complex, but can be a good way to work around the issue.