Wrapping a Closure so it is posted on a specific TaskRunner

46 views
Skip to first unread message

Jay Civelli

unread,
Feb 13, 2018, 3:35:19 PM2/13/18
to chromium-dev
Hi,
I ran couple of times recently into cases where I have a base::Closure that needs to be run from a specific TaskRunner. (e.g. the closure is a callback provided to a method run on the UI thread but then passed to the IO thread).
I was wondering if there were any existing utility method that I missed to wrap a base::Closure with a task runner so that when the wrapped closure is run, it posts a task on that passed in task runner.

Something like:

base::OnceClosure PostOnTaskRunner(
    base::OnceClosure closure,
    scoped_refptr<base::TaskRunner> task_runner) {
  return base::BindOnce([](base::OnceClosure closure,
                           scoped_refptr<base::TaskRunner> task_runner) {
    if (ThreadTaskRunnerHandle::Get() == task_runner) {
      std::move(closure).Run();
      return;
    }
    task_runner->PostTask(FROM_HERE, std::move(closure));
  }, std::move(closure), task_runner);
}

If it does not exist, would it make sense to add it? (to base/task_runner_util?)

Jay

Dale Curtis

unread,
Feb 13, 2018, 4:04:32 PM2/13/18
to Jay Civelli, chromium-dev
There's media::BindToCurrentLoop, https://cs.chromium.org/chromium/src/media/base/bind_to_current_loop.h

//base owners have had objections about moving it there: https://codereview.chromium.org/1083883003/

- dale

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAJ6%3Dx6nXFzqnUnTRiypU%2BZ2kGyRFRzeJzuV1d_ztdAh%2BO9aAdw%40mail.gmail.com.

Daniel Cheng

unread,
Feb 13, 2018, 4:50:50 PM2/13/18
to dalec...@chromium.org, Jay Civelli, chromium-dev
As Dale mentions, this is a topic that's come up a number of times before.

In the past, one idiom I've seen is to have a main class Foo and a helper class FooIOHelper. Each one stores the task runner to post tasks to (i.e. Foo owns the FooIOHelper and stores a ref to the IO thread task runner, and FooIOHelper holds a back pointer to Foo and stores a ref to the main thread task runner). I'm curious if this is something that would work for the instances you've encountered.

The advantages of this pattern are that the task runner restrictions are delineated by class boundaries and we don't need 2 callback template instantiations to trampoline a callback + its bound arguments back to the correct task runner.

However, it does require separate code out into separate classes which can be non-trivial. I'm also a bit unclear on how the proposed base::Promise API would handle cross-sequence promises, e.g. would it work in a similar way to BindToCurrentLoop today?

Daniel

Jay Civelli

unread,
Feb 13, 2018, 5:06:37 PM2/13/18
to Daniel Cheng, dalec...@chromium.org, chromium-dev
On Tue, Feb 13, 2018 at 1:49 PM, Daniel Cheng <dch...@chromium.org> wrote:
As Dale mentions, this is a topic that's come up a number of times before.

In the past, one idiom I've seen is to have a main class Foo and a helper class FooIOHelper. Each one stores the task runner to post tasks to (i.e. Foo owns the FooIOHelper and stores a ref to the IO thread task runner, and FooIOHelper holds a back pointer to Foo and stores a ref to the main thread task runner). I'm curious if this is something that would work for the instances you've encountered.
It would work for that case I was mentioning as an example (UI to IO thread call with Closure).
It does not work for another case I ran into, which is mojo::WrapCallbackWithDropHandler. This is a helper method that guarantees the callback is invoked when doing a Mojo interface call, even if the task is dropped, if the service crashed for example. (useful when you don't own the InterfacePtr and cannot use InterfacePtr::set_connection_error_handler) However that helper makes no guarantees on which thread the task will be run in that case, so BindToCurrentLoop would be pretty handy in that case.

Daniel Cheng

unread,
Feb 13, 2018, 5:22:05 PM2/13/18
to Jay Civelli, dalec...@chromium.org, chromium-dev
On Tue, Feb 13, 2018 at 2:05 PM Jay Civelli <jciv...@google.com> wrote:
On Tue, Feb 13, 2018 at 1:49 PM, Daniel Cheng <dch...@chromium.org> wrote:
As Dale mentions, this is a topic that's come up a number of times before.

In the past, one idiom I've seen is to have a main class Foo and a helper class FooIOHelper. Each one stores the task runner to post tasks to (i.e. Foo owns the FooIOHelper and stores a ref to the IO thread task runner, and FooIOHelper holds a back pointer to Foo and stores a ref to the main thread task runner). I'm curious if this is something that would work for the instances you've encountered.
It would work for that case I was mentioning as an example (UI to IO thread call with Closure).
It does not work for another case I ran into, which is mojo::WrapCallbackWithDropHandler. This is a helper method that guarantees the callback is invoked when doing a Mojo interface call, even if the task is dropped, if the service crashed for example. (useful when you don't own the InterfacePtr and cannot use InterfacePtr::set_connection_error_handler) However that helper makes no guarantees on which thread the task will be run in that case, so BindToCurrentLoop would be pretty handy in that case.

Can Mojo make the behavior in this case more deterministic by guaranteeing that it destroys the context of pending calls on the task runner bound to the interface pointer? It seems like that would be a generally useful thing to do.

Daniel

Alex Clarke

unread,
Feb 14, 2018, 4:07:54 AM2/14/18
to Daniel Cheng, dalec...@chromium.org, Jay Civelli, chromium-dev, Gabriel Charette
On 13 February 2018 at 21:49, Daniel Cheng <dch...@chromium.org> wrote:
As Dale mentions, this is a topic that's come up a number of times before.

In the past, one idiom I've seen is to have a main class Foo and a helper class FooIOHelper. Each one stores the task runner to post tasks to (i.e. Foo owns the FooIOHelper and stores a ref to the IO thread task runner, and FooIOHelper holds a back pointer to Foo and stores a ref to the main thread task runner). I'm curious if this is something that would work for the instances you've encountered.

The advantages of this pattern are that the task runner restrictions are delineated by class boundaries and we don't need 2 callback template instantiations to trampoline a callback + its bound arguments back to the correct task runner.

However, it does require separate code out into separate classes which can be non-trivial. I'm also a bit unclear on how the proposed base::Promise API would handle cross-sequence promises, e.g. would it work in a similar way to BindToCurrentLoop today?

The idea I'm kicking around for promises is a Then/Catch can optionally specify a sequence for the callback and the remainder of the promise chain to run on.  E.g. we could do this:

base::ManualPromiseResolver<void> mpr;
mpr.promise().Then(ui_sequenceFROM_HERE, base::Bind(&NextStep));
OldStyleApiWhichRunsStuffOnTheIoThread(mpr.GetResolveCallback());


Depending on what the API for the IO thread task looks like we could equally do this:
          io_task_runner->PostTask(FROM_HERE, base::Bind(&ThingToRunOnIoThread))
         .Then(ui_sequenceFROM_HERE, base::Bind(&ThingToTunOnUiThread));
        

 
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev+unsubscribe@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAF3XrKoWMB%2B%3D6Q%3DCvuRsE8X6e4qVzADQSf8VVsAcTAZaaCmKLg%40mail.gmail.com.

Claudio deSouza

unread,
Feb 14, 2018, 4:40:17 AM2/14/18
to Chromium-dev, dch...@chromium.org, dalec...@chromium.org, jciv...@google.com, g...@google.com
This looks interesting, and I would guess that with the migration to use sequences, this would lay the inevitable foundation for co-routines.

Gabriel Charette

unread,
Feb 14, 2018, 4:44:31 AM2/14/18
to Alex Clarke, Daniel Cheng, dalec...@chromium.org, Jay Civelli, chromium-dev
FWIW, I could have used such a helper a few times in the past and I'm sure we have many ad-hoc anonymous helpers to do this sequence hop, i.e.:

void MyCallBack(base::OnceClosure closure, scoped_refptr<base::SequencedTaskRunner> task_runner) {
  if (!task_runner->RunsTasksInCurrentSequence())
    task_runner->PostTask(FROM_HERE, std::move(closure);
  else
    std::move(closure).Run();
}

I think it would make sense to have such a helper in //base (with an optional destination task runner param that binds to the current sequence by default).

Also, without this we end up with many methods that take two params that are really a pair (i.e. a continuation Callback and a destination TaskRunner).
Looks like ≥25 use cases in our codebase from a simple regex.
The current proposal for Promises aims to support this natively in the API.

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.

Alex Clarke

unread,
Feb 14, 2018, 4:49:34 AM2/14/18
to claudi...@gmail.com, Chromium-dev, Daniel Cheng, dalec...@chromium.org, Jay Civelli, Gabriel Charette
I like the idea of co-routines and eventually being able to await for the result of a promise.  I expect in practice it may be rather difficult to get there.  For one thing clang doesn't support growable coroutine callstacks (although llvm does) although I expect it will do at some point judging by what Iv'e read about C++20. Another issue is tooling, we'd need to provide callstacks for each of the coroutines.

On 14 February 2018 at 09:40, Claudio deSouza <claudi...@gmail.com> wrote:
This looks interesting, and I would guess that with the migration to use sequences, this would lay the inevitable foundation for co-routines.

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev+unsubscribe@chromium.org.
Reply all
Reply to author
Forward
0 new messages