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

Integrating CurrentThread and foreign event loops #675

Closed
alexcrichton opened this issue Dec 7, 2017 · 9 comments
Closed

Integrating CurrentThread and foreign event loops #675

alexcrichton opened this issue Dec 7, 2017 · 9 comments

Comments

@alexcrichton
Copy link
Member

Today the CurrentThread API I think isn’t easily amenable to integration with a foreign event loop, such as the glib event loop in GTK. (a case I've forgotten about until now by accident!). The main gist of the problem is that right now the main way to customize the CurrentThread behavior is via the Sleep trait but that requires that Rust is the one which requests that the current thread block, whereas in a GTK/glib application Rust isn't the one doing that but C is.

I think we may want to add some form of nonblocking "move spawned tasks forward" API like:

pub struct TaskRunner { /* ... */ }

impl TaskRunner {
    pub fn new() -> TaskRunner;

    // executes a closure where this task runner is the "ambient"
    // task runner where tasks are spawned
    //
    // panics if called recursively
    pub fn set_default(&mut self, f: impl FnOnce());

    // In a nonblocking fashion pushes all tasks forward
    //
    // if a task isn't ready then `wakeup` will be notified when it is
    //
    // panics if called within `set_default` (can't recurse)
    pub fn poll(&mut self, wakeup: SomeWakeupHandle);
}

With an API like that we could update the existing APIs behaviorally like:

pub struct CurrentThread { /* ... */ }

impl CurrentThread {
    // These functions will route to a `TaskRunner` if configured via 
    // `set_default` or `poll`.
    pub fn execute(f: impl Future<Item = (), Error = ()>);
    pub fn execute_daemon(f: impl Future<Item = (), Error = ()>);
    pub fn cancel_all_executing();

    // In addition to the current panic conditions, these functions will 
    // panic if `poll` or `set_default` above was previously called.
    pub fn run<R>(f: impl FnOnce(&mut Context) -> R) -> R;
    pub fn run_with_sleep<R>(s: &mut impl Sleep, 
                             f: impl FnOnce(&mut Context) -> R) -> R;
}

Effectively we'd just export some of the internals of the current thread module to interact a little more closely with external systems. We'd then require foreign event loop integration to look like:

  • When booted they'd create a TaskRunner
  • Whenever Rust code is executed they execute set_default to get spawning to work
  • When ready, the TaskRunner is polled with the ability to route wakeups to the glib event loop

I think this'd work for glib integration, but curious what others think!

@aturon
Copy link
Member

aturon commented Dec 7, 2017

cc @carllerche re: reform branch.

@carllerche
Copy link
Member

but that requires that Rust is the one which requests that the current thread block, whereas in a GTK/glib application Rust isn't the one doing that but C is.

I'm not sure why this is true as one could impl the Sleep trait using extern calls? Perhaps in the GTK/glib case, not enough APIs are provided to implement Sleep?

Perhaps it would help for me to understand this better if you could explain how this API would be used with glib / gtk?

That said, at a high level, something like this seems important. I would just like to have a better sense of how it all fits together.

@alexcrichton
Copy link
Member Author

Er sorry yeah so what I mean with the glib case is that you have very little control at all. For example Rust as a language integrating into a foreign event loop (like glib) can't block at all (or request to block). It's sort of like if you bolted a tokio-reactor onto a tokio-reactor the sub-reactor wouldn't be able to block, it just knows when it needs to get polled and it can't as the outer reactor to block because that's managed elsewhere.

Does that make sense?

@carllerche
Copy link
Member

Yeah, roughly. I see why it is needed at a high level. I’m just not exactly sure how the proposed api fits in with something like glib/gtk.

I would assume that the argument to poll would take T:Wakeup. What glib fn would that call?

Also, in terms of naming bikeshedding (and this is not really related to just this issue), set_default as a name makes it sound like it is a more permanent action and not just for the context of a closure.

“with” or “enter” as terms seems more fitting as terms.

Anyway, if you want to add this api it seems fine to me. The docs should de-emphasize using it in favor of other options and only reach to this api when necessary for integrating with other loops.

alexcrichton added a commit that referenced this issue Jan 5, 2018
This commit extracts the `TaskRunner` interface from the `current_thread` module
to a standalone interface and also reexports it from the `executor` module. This
is done with #675 as the primary motivation, namely accommodating crates using
`CurrentThread` on foreign event loops like glib. In this situation Rust (and
associated code) can't request to block (aka with `run_with_sleep`) and as a
result need a nonblocking method (`TaskRunner::poll` here) instead.

The `current_thread` module was then reimplemented in terms of `TaskRunner` to
ensure there's no extra duplication.
alexcrichton added a commit that referenced this issue Jan 8, 2018
This commit extracts the `TaskRunner` interface from the `current_thread` module
to a standalone interface and also reexports it from the `executor` module. This
is done with #675 as the primary motivation, namely accommodating crates using
`CurrentThread` on foreign event loops like glib. In this situation Rust (and
associated code) can't request to block (aka with `run_with_sleep`) and as a
result need a nonblocking method (`TaskRunner::poll` here) instead.

The `current_thread` module was then reimplemented in terms of `TaskRunner` to
ensure there's no extra duplication.
@carllerche
Copy link
Member

Going back to the original problem of integrating with a foreign event loop.

It seems that code that actually needs to do this is limited to libs that provide bindings for this foreign event loop. This is not to say it is an unimportant case.

I wonder if instead of handling all possible current thread executor cases, we instead provide one for the common cases and also provide a way to hook into the CurrentThread::execute APIs, i.e. provide the ability to customize the executor outside of the futures-rs crate.

This would provide the added benefit of being able to bring your own implementation, optimized for your needs.

I don't think doing this would be particularly hard, there would just need to be a way to pass a &mut Executor<Box<Future<Item = (), Error = ()>>> to set the thread-local variable:

type UnsyncFuture = Box<Future<Item = (), Error = ()>>;

fn with_my_custom_executor<F, R>(&mut Executor<UnsyncFuture>, f: F) -> R
    where F: FnOnce(&mut Executor<UnsyncFuture>) -> R,
{
    // ...
}

@alexcrichton
Copy link
Member Author

Yes I think that would also solve the constraint of allowing integration into foreign event loops while allowing libraries to use CurrentThread.

@carllerche
Copy link
Member

As I mentioned in #717, I would suggest punting on this issue for the initial "tokio reform" release.

@aturon
Copy link
Member

aturon commented Jan 31, 2018

Agreed; I've untagged.

@aturon
Copy link
Member

aturon commented Feb 12, 2018

I'm going to close this out in favor of the executor RFC

@aturon aturon closed this as completed Feb 12, 2018
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