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

Repaint signal cannot be shared between threads #82

Closed
DRvader opened this issue Dec 27, 2020 · 5 comments
Closed

Repaint signal cannot be shared between threads #82

DRvader opened this issue Dec 27, 2020 · 5 comments

Comments

@DRvader
Copy link

DRvader commented Dec 27, 2020

Hey,

I've been having problems sharing the repaint signal between threads. It seems the problem is that Arc requires that it wraps types that implement send and sync. Though I am new enough to rust that it could also be my fault.

The following code (in the ui function) will cause the error.

let repaint_signal = integration_context.repaint_signal.clone();
let thread = std::thread::spawn(|| {
  repaint_signal.request_repaint();
});
thread.join();

I'm happy to fix this issue if the problem wasn't caused by my own lack of understanding.

@emilk
Copy link
Owner

emilk commented Dec 27, 2020

Hi!

RepaintSignal is not meant to be shared between threads (which would require Sync), instead it is meant to be cloned (as you do) and then moved to the other thread (requiring just Send). You are almost there with your code, you just lack a move:

let thread = std::thread::spawn(move || {
  repaint_signal.request_repaint();
});

That should work!

@DRvader
Copy link
Author

DRvader commented Dec 27, 2020

Thanks for getting back to me 😄

I tried adding the move as you suggested but that didn't seem to fix the problem.

The error message that cargo spits out seemed to say that Arc requires both Send and Sync to be implemented for the type it contains.

This seems to agree with the source for Arc that I found in the rust-doc

#[stable(feature = "rust1", since = "1.0.0")]
unsafe impl<T: ?Sized + Sync + Send> Send for Arc<T> {}
#[stable(feature = "rust1", since = "1.0.0")]
unsafe impl<T: ?Sized + Sync + Send> Sync for Arc<T> {}

I don't understand enough about the internals of rust to be even remotely confident that I didn't make an unrelated mistake or messed up my build environment somehow so please let me know if I've gone wrong somewhere.

I've also attached the error message and the file I used to generate it so you can let me know if I'm interpreting the output correctly.

error[E0277]: `dyn RepaintSignal` cannot be shared between threads safely
   --> src\main.rs:12:22
    |
12  |         let thread = std::thread::spawn(move || {
    |                      ^^^^^^^^^^^^^^^^^^ `dyn RepaintSignal` cannot be shared between threads safely
    | 
   ::: C:\Users\micro\.rustup\toolchains\stable-x86_64-pc-windows-msvc\lib/rustlib/src/rust\library\std\src\thread\mod.rs:607:8
    |
607 |     F: Send + 'static,
    |        ---- required by this bound in `spawn`
    |
    = help: the trait `Sync` is not implemented for `dyn RepaintSignal`
    = note: required because of the requirements on the impl of `Send` for `Arc<dyn RepaintSignal>`
    = note: required because it appears within the type `[closure@src\main.rs:12:41: 14:10]`

error: aborting due to previous error
use egui::{app::IntegrationContext, CtxRef};

struct App {}

impl egui::app::App for App {
    fn name(&self) -> &str {
        "Name"
    }

    fn ui(&mut self, ctx: &CtxRef, integration_context: &mut IntegrationContext) {
        let repaint_signal = integration_context.repaint_signal.clone();
        let thread = std::thread::spawn(move || {
            repaint_signal.request_repaint();
        });
        thread.join();
    }
}

fn main() {
    egui_glium::run(Box::new(App {}))
}

@emilk
Copy link
Owner

emilk commented Dec 27, 2020

You are of course correct! I will come with a fix shortly. And thanks for your report!

@emilk emilk closed this as completed in 958fc27 Dec 27, 2020
@emilk
Copy link
Owner

emilk commented Dec 27, 2020

You should now be able to continue if you add the following patch to your Cargo.toml:

[patch.crates-io]
egui = { git = "https://github.com/emilk/egui", rev = "958fc2753aef7e35b615a13c54324860ee7f1427" } # 2020-12-27
egui_glium = { git = "https://github.com/emilk/egui", rev = "958fc2753aef7e35b615a13c54324860ee7f1427" } # 2020-12-27
egui_web = { git = "https://github.com/emilk/egui", rev = "958fc2753aef7e35b615a13c54324860ee7f1427" } # 2020-12-27

Good luck!

@DRvader
Copy link
Author

DRvader commented Dec 27, 2020

Great! Thanks for the help 😁

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

2 participants