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

MacOSX' RecommendedWatcher (FsEventWatcher) doesn't implement Send #82

Closed
julienw opened this issue Jun 23, 2016 · 6 comments
Closed

MacOSX' RecommendedWatcher (FsEventWatcher) doesn't implement Send #82

julienw opened this issue Jun 23, 2016 · 6 comments

Comments

@julienw
Copy link
Contributor

julienw commented Jun 23, 2016

This is misleading because other watchers are Send and so can be moved between threads.

Here is an example of an error I get in MacOS X only:

src/lib.rs:643:9: 643:22 error: the trait bound `*mut libc::c_void: std::marker::Send` is not satisfied [E0277]
src/lib.rs:643         thread::spawn(move || {
                                                                                                          ^~~~~~~~~~~~~
src/lib.rs:643:9: 643:22 help: run `rustc --explain E0277` to see a detailed explanation
src/lib.rs:643:9: 643:22 note: `*mut libc::c_void` cannot be sent between threads safely
src/lib.rs:643:9: 643:22 note: required because it appears within the type `notify::FsEventWatcher`
src/lib.rs:643:9: 643:22 note: required because it appears within the type `FsWatcher`
@julienw
Copy link
Contributor Author

julienw commented Jun 23, 2016

This is not obvious, but cf::CFMutableArrayRef is an alias to *mut libc::c_void.

@julienw
Copy link
Contributor Author

julienw commented Jun 24, 2016

I think we could add unsafe impl Send for FsEventWatcher, as it seems safe to pass this pointer accross threads. Not sure for Sync, but I think it would be fine as well because the methods that change states use &mut self.

What do you think ?

If you agree I can write the patch.

@passcod
Copy link
Member

passcod commented Jun 24, 2016

Cc @octplane

@octplane
Copy link
Contributor

@julienw that would be nice if you could write the patch. Does that only consist in adding the unsafe.. to the impl ? Would you mind trying to write a test for that too ?

@julienw
Copy link
Contributor Author

julienw commented Jun 26, 2016

Yes, it would be simply adding the unsafe impl Send//Sync. I can try to write a test :)

@octplane
Copy link
Contributor

Nice 🌅 . Let us know if anything is blocking you.

julienw added a commit to julienw/rsnotify that referenced this issue Jun 28, 2016
julienw added a commit to julienw/rsnotify that referenced this issue Jun 28, 2016
@passcod passcod closed this as completed in 10e5e24 Jul 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants