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

Add a Futures Executor around MainContext and various Future/Stream implementations #314

Merged
merged 12 commits into from Apr 19, 2018

Conversation

Projects
None yet
3 participants
@sdroege
Member

sdroege commented Apr 16, 2018

This Executor will also be used in GIO later for Futures around the async operations (and GSources)

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Apr 16, 2018

Member

cargo build --features futures on Windows

error[E0277]: the trait bound `*mut libc::c_void: std::marker::Send` is not satisfied in `[closure@src\source_futures.rs:157:23: 162:6 pid:source::Pid, priority:source::Priority]`
   --> src\source_futures.rs:157:5
    |
157 |     SourceFuture::new(move |send| {
    |     ^^^^^^^^^^^^^^^^^ `*mut libc::c_void` cannot be sent between threads safely
    |
    = help: within `[closure@src\source_futures.rs:157:23: 162:6 pid:source::Pid, priority:source::Priority]`, the trait `std::marker::Send` is not implemented for `*mut libc::c_void`
    = note: required because it appears within the type `source::Pid`
    = note: required because it appears within the type `[closure@src\source_futures.rs:157:23: 162:6 pid:source::Pid, priority:source::Priority]`
note: required by `<source_futures::SourceFuture<F, T>>::new`
   --> src\source_futures.rs:31:5
    |
31  |     pub fn new(create_source: F) -> Box<Future<Item = T, Error = Never>> {
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0277]: the trait bound `*mut libc::c_void: std::marker::Send` is not satisfied in `std::option::Option<(source::Pid, i32)>`
   --> src\source_futures.rs:159:9
    |
159 |         ::child_watch_source_new(pid, None, priority, move |pid, code| {
    |         ^^^^^^^^^^^^^^^^^^^^^^^^ `*mut libc::c_void` cannot be sent between threads safely
    |
    = help: within `std::option::Option<(source::Pid, i32)>`, the trait `std::marker::Send` is not implemented for `*mut libc::c_void`
    = note: required because it appears within the type `source::Pid`
    = note: required because it appears within the type `(source::Pid, i32)`
    = note: required because it appears within the type `std::option::Option<(source::Pid, i32)>`
    = note: required because of the requirements on the impl of `std::marker::Send` for `futures_channel::lock::Lock<std::option::Option<(source::Pid, i32)>>`
    = note: required because it appears within the type `futures_channel::oneshot::Inner<(source::Pid, i32)>`
    = note: required because of the requirements on the impl of `std::marker::Send` for `std::sync::Arc<futures_channel::oneshot::Inner<(source::Pid, i32)>>`
    = note: required because it appears within the type `futures_channel::oneshot::Sender<(source::Pid, i32)>`
    = note: required because it appears within the type `std::option::Option<futures_channel::oneshot::Sender<(source::Pid, i32)>>`
    = note: required because it appears within the type `[closure@src\source_futures.rs:159:55: 161:10 send:std::option::Option<futures_channel::oneshot::Sender<(source::Pid, i32)>>]`
note: required by `source::child_watch_source_new`
   --> src\source.rs:352:1
    |
352 | / pub fn child_watch_source_new<'a, N: Into<Option<&'a str>>, F>(pid: Pid, name: N, priority: Priority, func: F) -> Source
353 | | where F: FnMut(Pid, i32) + Send + 'static {
354 | |     unsafe {
355 | |         let source = glib_ffi::g_child_watch_source_new(pid.0);
...   |
366 | |     }
367 | | }
    | |_^

error: aborting due to 2 previous errors
Member

EPashkin commented Apr 16, 2018

cargo build --features futures on Windows

error[E0277]: the trait bound `*mut libc::c_void: std::marker::Send` is not satisfied in `[closure@src\source_futures.rs:157:23: 162:6 pid:source::Pid, priority:source::Priority]`
   --> src\source_futures.rs:157:5
    |
157 |     SourceFuture::new(move |send| {
    |     ^^^^^^^^^^^^^^^^^ `*mut libc::c_void` cannot be sent between threads safely
    |
    = help: within `[closure@src\source_futures.rs:157:23: 162:6 pid:source::Pid, priority:source::Priority]`, the trait `std::marker::Send` is not implemented for `*mut libc::c_void`
    = note: required because it appears within the type `source::Pid`
    = note: required because it appears within the type `[closure@src\source_futures.rs:157:23: 162:6 pid:source::Pid, priority:source::Priority]`
note: required by `<source_futures::SourceFuture<F, T>>::new`
   --> src\source_futures.rs:31:5
    |
31  |     pub fn new(create_source: F) -> Box<Future<Item = T, Error = Never>> {
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0277]: the trait bound `*mut libc::c_void: std::marker::Send` is not satisfied in `std::option::Option<(source::Pid, i32)>`
   --> src\source_futures.rs:159:9
    |
159 |         ::child_watch_source_new(pid, None, priority, move |pid, code| {
    |         ^^^^^^^^^^^^^^^^^^^^^^^^ `*mut libc::c_void` cannot be sent between threads safely
    |
    = help: within `std::option::Option<(source::Pid, i32)>`, the trait `std::marker::Send` is not implemented for `*mut libc::c_void`
    = note: required because it appears within the type `source::Pid`
    = note: required because it appears within the type `(source::Pid, i32)`
    = note: required because it appears within the type `std::option::Option<(source::Pid, i32)>`
    = note: required because of the requirements on the impl of `std::marker::Send` for `futures_channel::lock::Lock<std::option::Option<(source::Pid, i32)>>`
    = note: required because it appears within the type `futures_channel::oneshot::Inner<(source::Pid, i32)>`
    = note: required because of the requirements on the impl of `std::marker::Send` for `std::sync::Arc<futures_channel::oneshot::Inner<(source::Pid, i32)>>`
    = note: required because it appears within the type `futures_channel::oneshot::Sender<(source::Pid, i32)>`
    = note: required because it appears within the type `std::option::Option<futures_channel::oneshot::Sender<(source::Pid, i32)>>`
    = note: required because it appears within the type `[closure@src\source_futures.rs:159:55: 161:10 send:std::option::Option<futures_channel::oneshot::Sender<(source::Pid, i32)>>]`
note: required by `source::child_watch_source_new`
   --> src\source.rs:352:1
    |
352 | / pub fn child_watch_source_new<'a, N: Into<Option<&'a str>>, F>(pid: Pid, name: N, priority: Priority, func: F) -> Source
353 | | where F: FnMut(Pid, i32) + Send + 'static {
354 | |     unsafe {
355 | |         let source = glib_ffi::g_child_watch_source_new(pid.0);
...   |
366 | |     }
367 | | }
    | |_^

error: aborting due to 2 previous errors
@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Apr 16, 2018

Member

Ah good. @EPashkin are pids Send on Windows? I assume they are, just that we don't express them as such currently?

Member

sdroege commented Apr 16, 2018

Ah good. @EPashkin are pids Send on Windows? I assume they are, just that we don't express them as such currently?

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Apr 16, 2018

Member

I've added a commit to that effect

Member

sdroege commented Apr 16, 2018

I've added a commit to that effect

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Apr 16, 2018

Member

@sdroege https://developer.gnome.org/glib/stable/glib-The-Main-Event-Loop.html#GPid
says that process id on windows is Handle, and IMHO it Send.

Member

EPashkin commented Apr 16, 2018

@sdroege https://developer.gnome.org/glib/stable/glib-The-Main-Event-Loop.html#GPid
says that process id on windows is Handle, and IMHO it Send.

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Apr 16, 2018

Member

Build and test with --features futures passed for me locally. Thanks

Member

EPashkin commented Apr 16, 2018

Build and test with --features futures passed for me locally. Thanks

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Apr 16, 2018

Member

Ok, then we only need someone to review all this and then can get it merged :)

Member

sdroege commented Apr 16, 2018

Ok, then we only need someone to review all this and then can get it merged :)

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Apr 16, 2018

Member

@sdroege IMHO PR looks very good, Thanks.

Member

EPashkin commented Apr 16, 2018

@sdroege IMHO PR looks very good, Thanks.

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Apr 17, 2018

Member

Hrm, due to moving from impl trait to boxed futures this lost the ability to have Send implemented on the resulting future automatically. Will try to find a solution for that.

EDIT: Done, was simple fortunately

Member

sdroege commented Apr 17, 2018

Hrm, due to moving from impl trait to boxed futures this lost the ability to have Send implemented on the resulting future automatically. Will try to find a solution for that.

EDIT: Done, was simple fortunately

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Apr 19, 2018

Member

@GuillaumeGomez I confirming that examples builds fine with this PR,
on test on CI is same path problem,
so seems it time to merge this.

Member

EPashkin commented Apr 19, 2018

@GuillaumeGomez I confirming that examples builds fine with this PR,
on test on CI is same path problem,
so seems it time to merge this.

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Apr 19, 2018

Member

Thanks, let's go!

Member

GuillaumeGomez commented Apr 19, 2018

Thanks, let's go!

@GuillaumeGomez GuillaumeGomez merged commit 363f07a into gtk-rs:master Apr 19, 2018

1 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment