Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can the fluid world protected by a arc mutex be shared between threads? #6

Open
ajunca opened this issue Apr 26, 2020 · 5 comments
Open

Comments

@ajunca
Copy link

ajunca commented Apr 26, 2020

I'm testing nphysics, salva and legion (entity component system) together. I need all the physics data to be a legion resource (and that it means that it will be shared between threads). The problem is with the LiquidWorld that is non Send, so I can not included inside the physics bundle resource. If I wrap everything with a Arc<Mutex<>> like the following code:

pub struct PhysicsResourceInner {
    pub mechanical_world: DefaultMechanicalWorld<Real>,
    pub geometrical_world: DefaultGeometricalWorld<Real>,
    pub bodies: DefaultBodySet<Real>,
    pub colliders: DefaultColliderSet<Real>,
    pub joint_constraints: DefaultJointConstraintSet<Real>,
    pub force_generators: DefaultForceGeneratorSet<Real>,

    // Fluid
    pub fluid_world: LiquidWorld::<Real>,
    pub fluid_couplings: ColliderCouplingSet::<Real, DefaultBodyHandle>,
}

#[derive(Clone)]
pub struct PhysicsResource {
    pub inner: Arc<Mutex<PhysicsResourceInner>>,
}

Then I can mark it as Send using an "unsafe" impl (like the above). Is this really safe or there is some hidden problems that will arise?
unsafe impl Send for PhysicsResource {}

EDIT: Not even the unsafe impl makes it compile. The important part of the error messages goes like:

error[E0277]: `(dyn salva2d::solver::pressure::pressure_solver::PressureSolver<f32> + 'static)` cannot be sent between threads safely
   --> src/app/mod.rs:106:73
    |
106 |                     if let Some(physics_resource) = &mut self.resources.get_mut::<PhysicsResource>() {
    |                                                                         ^^^^^^^ `(dyn salva2d::solver::pressure::pressure_solver::PressureSolver<f32> + 'static)` cannot be sent between threads safely
    |
    = help: the trait `std::marker::Send` is not implemented for `(dyn salva2d::solver::pressure::pressure_solver::PressureSolver<f32> + 'static)`
    = note: required because of the requirements on the impl of `std::marker::Send` for `std::ptr::Unique<(dyn salva2d::solver::pressure::pressure_solver::PressureSolver<f32> + 'static)>`
    = note: required because it appears within the type `std::boxed::Box<(dyn salva2d::solver::pressure::pressure_solver::PressureSolver<f32> + 'static)>`
    = note: required because it appears within the type `salva2d::liquid_world::LiquidWorld<f32>`
    = note: required because it appears within the type `resources::physics::PhysicsResourceInner`
    = note: required because of the requirements on the impl of `std::marker::Send` for `std::sync::Mutex<resources::physics::PhysicsResourceInner>`
    = note: required because of the requirements on the impl of `std::marker::Sync` for `std::sync::Arc<std::sync::Mutex<resources::physics::PhysicsResourceInner>>`
    = note: required because it appears within the type `resources::physics::PhysicsResource`
    = note: required because of the requirements on the impl of `legion_systems::resource::Resource` for `resources::physics::PhysicsResource` 

Edit 2: If I change on the salva source "liquid_world.rs" (the two lines above), I can compile (without the unsafe impl). But I'm not sure if it ok or I'm missing something. Above the changes I made:

On the LiquidWorld struct definition add + Send to the solve:
solver: Box<dyn PressureSolver<N> + Send>,

And also add + Send to the new function:
solver: impl PressureSolver<N> + 'static + Send,

@ajunca ajunca changed the title Can the IISPH solver protected by a arc mutex be shared between threads? Can the fluid world protected by a arc mutex be shared between threads? Apr 26, 2020
@LoganDark
Copy link

LiquidWorld is not Sync, so you should not share it between threads at all. Only one thread may own and interact with the LiquidWorld. Using unsafe to bypass this behavior is, well, unsafe.

@anlumo
Copy link

anlumo commented Mar 11, 2021

Sharing between threads is done using the Send trait, not Sync. It's not Send either though.

@LoganDark
Copy link

Sharing between threads is done using the Send trait, not Sync. It's not Send either though.

Actually, sharing between threads is done by Sending a shared reference:

A type is Sync if it is safe to share between threads (T is Sync if and only if &T is Send).

If LiquidWorld was Send, you would be able to transfer it between threads, but not use it from multiple threads simultaneously.

@anlumo
Copy link

anlumo commented Mar 11, 2021

True, not at the same time, but you could wrap it in an Arc<Mutex<LiquidWorld>> and use it sequentially from multiple threads.

@LoganDark
Copy link

True, not at the same time, but you could wrap it in an Arc<Mutex<LiquidWorld>> and use it sequentially from multiple threads.

That's true, because Arc is Send and Mutex is Sync, emulating the exact same behavior mentioned above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants