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

Watcher isn't Sync with --no-default-features #503

Closed
jamessan opened this issue Jul 7, 2023 · 1 comment · Fixed by #523
Closed

Watcher isn't Sync with --no-default-features #503

jamessan opened this issue Jul 7, 2023 · 1 comment · Fixed by #523
Labels
B-doc improvement/fix for documentation

Comments

@jamessan
Copy link

jamessan commented Jul 7, 2023

Note: we may want to document and test for this, so it isn't reverted later on with a trait under the impression of usability improvements

I did add a simple test for this in src/lib.rs, where I assert that the Watcher trait is object safe. So we can't accidentally remove this feature.

Originally posted by @erickt in #336 (comment)

That only works when not using std::sync::mpsc::Sender:

$ cargo test -p notify --no-default-features
   Compiling notify v6.0.1 (notify/notify)
error[E0277]: `std::sync::mpsc::Sender<EventLoopMsg>` cannot be shared between threads safely
   --> notify/src/inotify.rs:570:13
    |
570 |     check::<INotifyWatcher>();
    |             ^^^^^^^^^^^^^^ `std::sync::mpsc::Sender<EventLoopMsg>` cannot be shared between threads safely
    |
    = help: within `INotifyWatcher`, the trait `Sync` is not implemented for `std::sync::mpsc::Sender<EventLoopMsg>`
note: required because it appears within the type `INotifyWatcher`
   --> notify/src/inotify.rs:45:12
    |
45  | pub struct INotifyWatcher {
    |            ^^^^^^^^^^^^^^
note: required by a bound in `inotify::inotify_watcher_is_send_and_sync::check`
   --> notify/src/inotify.rs:569:24
    |
569 |     fn check<T: Send + Sync>() {}
    |                        ^^^^ required by this bound in `inotify::inotify_watcher_is_send_and_sync::check`

For more information about this error, try `rustc --explain E0277`.
error: could not compile `notify` due to previous error
@0xpr03 0xpr03 added the A-bug label Jul 7, 2023
@0xpr03
Copy link
Member

0xpr03 commented Jul 12, 2023

I don't see a way around this. If someone needs this to be Sync, they'll have to use crossbeam-channel. Should probably document this.

@0xpr03 0xpr03 added B-doc improvement/fix for documentation and removed A-bug labels Jul 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-doc improvement/fix for documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants