Unifying Chromium's Callback Systems

[Albert J. Wong (王重傑)]

Andrew (scherkus@) and I got together a few weeks ago to try and take a high-level look at our Callback system in Chromium.  The primary motivator has been our experience trying to use the system in the media and chromoting code bases.  One key observation was that we have too many callback systems, and none quite do everything you'd want. In an attempt to rationalize this, here is a design doc describing the state of the world, a proposal for what it could look like, and an outline for how to migrate there.

   https://docs.google.com/document/edit?id=1fwVk_nM9mxKzvOyuXFNVnueb9hrC7RVJ2RT1T7tCoyQ&hl=en#heading=h.zbuv8kliby3b

Here's is a WIP attempt at modifying stoyan's "mutant callback" generator to do what is described in the doc above.

   http://codereview.chromium.org/3555014

Note that both the doc and the CL are works in progress.  We're both scraping out 20% time to play with this, so progress is slow.

Comments are definitely welcome.

Thanks,

Albert

[Matt Perry]

The Task/Callback system has been a mess for a while now. It's awesome to see that someone has finally stepped up to fix it!

I like the proposal for the most part. I have one nit, and one bigger concern. (The doc is not editable, responses here.)

nit: ResultCallback vs Callback - it seems like the "return void" problem could be resolved using template specialization. But it might not be a big deal either way.

bigger concern: I have a feeling the current traits-based way of specifying refcounting semantics is safer than relying on New[Ref]Callback. With traits, the compiler warns you if you try to use a non-refcounted class with PostTask, and you have to take explicit action to say "no, this is really what I want." It sounds like you would have no such warning with the current proposal; that is, you could mix calls to NewCallback(classFoo) and NewRefCallback(classFoo). It would be nice if the refcounting semantics could be specified statically (and checked at compile time) rather than via a virtual function.

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

[Adam Barth]

I didn't see mentioned in the doc how you plan to support the
(non-RefCounted) use cases of ScopedRunnableMethodFactory. Are you
planning to port ScopedMumbleFactory over to the new callback system?

Adam

On Mon, Oct 4, 2010 at 2:58 PM, Albert J. Wong (王重傑)
<ajw...@chromium.org> wrote:

[Peter Kasting]

On Mon, Oct 4, 2010 at 2:58 PM, Albert J. Wong (王重傑) <ajw...@chromium.org> wrote:

Andrew (scherkus@) and I got together a few weeks ago to try and take a high-level look at our Callback system in Chromium.  The primary motivator has been our experience trying to use the system in the media and chromoting code bases.  One key observation was that we have too many callback systems, and none quite do everything you'd want. In an attempt to rationalize this, here is a design doc describing the state of the world, a proposal for what it could look like, and an outline for how to migrate there.

   https://docs.google.com/document/edit?id=1fwVk_nM9mxKzvOyuXFNVnueb9hrC7RVJ2RT1T7tCoyQ&hl=en#heading=h.zbuv8kliby3b

Are Timers suitable for consideration here too?

PK 

[Fred Akalin]

On Mon, Oct 4, 2010 at 2:58 PM, Albert J. Wong (王重傑)
<ajw...@chromium.org> wrote:

> Andrew (scherkus@) and I got together a few weeks ago to try and take a
> high-level look at our Callback system in Chromium.

Cool!

Did you guys look at TR1 function/bind? They may be mature enough
nowadays that it *may* be feasible to implement a compatible callback
system using it. On the other hand, it may be more trouble than it's
worth.

[John Abd-El-Malek]

Also, one feature that'd be great to have would be to use something like ScopedRunnableMethodFactory to run methods on a (non-refcounted) object on different threads.  This is needed in a bunch of places and is hacked around.  Having a clean way of doing this would be very nice.  One potential way of doing this would be to have it include a lock, but only start using it if a method is posted to a different thread.

[Albert J. Wong (王重傑)]

Thanks for the questions and feedback so far.  Going to respond to everyone in one mail.

Matt Perry:

ResultCallback annoyed me too.  I'll play with specializations a bit to see if it can be done.  There may be a symbol space trade-off there though since you add yet another template parameter.

As for RunnableMethodTraits, and usage with PostTask, Brett commented on this earlier.  Currently, I was suggesting 2 different functions in MessageLoop, PostTask() and PostClosure().  PostTask() would only take an instance of RefClosure which gives the static type checking.  An argument could be made to rename PostClosure into something scarier like PostUnsafeTask().  Would that resolve the concern?

Note I haven't yet examined all the RunnableMethodTraits yet, so there might be a case I'm missing.  But for just the static checking on PostTask, I think the above will work around well enough.

Adam Barth, John Abd-El-Malek:

I completely forgot about ScopedRunnableMethodFactory.  I've actually never used this myself.  Am I right that the usecase here is largely to be able to take a whole set of callbacks and "detach" them when an object dies?

If so, we should be able to implement something very similar on top of this.

Fred Akalin:

So TR1 function/bind probably would work.  I had excluded them from consideration because, at least on gcc-4.2, including tr1/function ends up requiring RTTI.  We went through this pain when trying to include gmock a while back, and zhanyong actually ended up implementing a custom Tuple class to handle this.

If you think TR1 is worth pursing, I think the first question to answer is if we can make the toolchain do what we want.

-Albert

P.S.  If people actually want to have edit access to the doc, let me know. I'm being a bit restrictive because I'd rather have the discussion on the mailing list than in Docs comments since those have a tendency to get unreadable.  E-mail archives are also easier to search.

[Albert J. Wong (王重傑)]

Agh...missed Peter.

Looking through base/timer.h, I we can just port the Timer API over to the analogous new callback types and things should continue to work.  Were you thinking of some sort of tighter integration?

-Albert

[Adam Barth]

On Mon, Oct 4, 2010 at 5:19 PM, Albert J. Wong (王重傑)
<ajw...@chromium.org> wrote:
> Adam Barth, John Abd-El-Malek:
> I completely forgot about ScopedRunnableMethodFactory.  I've actually never
> used this myself.  Am I right that the usecase here is largely to be able to
> take a whole set of callbacks and "detach" them when an object dies?
> If so, we should be able to implement something very similar on top of this.

ScopedRunnableMethodFactory is for objects that aren't RefCounted.
When the object dies, all the in-flight callbacks are cancelled.

Adam

[John Abd-El-Malek]

and it only works to invoke a method on the same thread.  doing so on other threads would be great.  I think it's not so difficult changing ScopedRunnableMethodFactory  to do so, but I haven't had a chance.  I was planning on doing it soon, but if you guys are rewriting all this, then I'll just wait :)

Adam

[Matt Perry]

On Mon, Oct 4, 2010 at 5:19 PM, Albert J. Wong (王重傑) <ajw...@chromium.org> wrote:

Thanks for the questions and feedback so far.  Going to respond to everyone in one mail.

Matt Perry:

ResultCallback annoyed me too.  I'll play with specializations a bit to see if it can be done.  There may be a symbol space trade-off there though since you add yet another template parameter.

I don't see why there needs to be anther template parameter. I was thinking of something like:

template<typename R, typename A>

class Callback1 {

  virtual R Run(A a);

};

// specialization for void

template<typename A>

class Callback1<void, A> {

  virtual void Run(A a);

};

As for RunnableMethodTraits, and usage with PostTask, Brett commented on this earlier.  Currently, I was suggesting 2 different functions in MessageLoop, PostTask() and PostClosure().  PostTask() would only take an instance of RefClosure which gives the static type checking.  An argument could be made to rename PostClosure into something scarier like PostUnsafeTask().  Would that resolve the concern?

I see. The separate PostFoo methods might be enough safety here (as long as you name them appropriately).

[Albert J. Wong (王重傑)]

I see.  I think we could largely just modify the existing impelmentation to use the new types, no?

I'm also tempted to suggest an alternate API like "CallbackCanceller" that might behave as follows.

CallbackCanceller canceller;

MessageLoop.PostTask(canceller.MonitorTask(NewCallback(&obj, &Foo::method1));

MessageLoop.PostTask(canceller.MonitorTask(NewCallback(&obj, &Foo::method2));

That would create a callback wrapper that the message loop could invoke.  This avoids the need to fork off the factory methods.

This unfortunately would 2x the memory usage, and add an additional function call. Not sure how big of a deal that really is...

-Albert

[Albert J. Wong (王重傑)]

Matt Perry:

ResultCallback annoyed me too.  I'll play with specializations a bit to see if it can be done.  There may be a symbol space trade-off there though since you add yet another template parameter.

I don't see why there needs to be anther template parameter. I was thinking of something like:

Right..you could take the ResultCallback and super-set the functionality of the void-Callbacks. The difference is that void-Callback types actually don't have a "typename R" so by unifying the two, we've increased the type list for all the void-Callbacks by 1.

John Abd-El-Malek:

You probably shouldn't wait on this work. :)  Realistically, unless I get a serious patch of free time or someone else picks it up, it's going to be months before this gets rolled out.

-Albert

[Peter Kasting]

On Mon, Oct 4, 2010 at 5:30 PM, Albert J. Wong (王重傑) <ajw...@chromium.org> wrote:

Agh...missed Peter.

Looking through base/timer.h, I we can just port the Timer API over to the analogous new callback types and things should continue to work.  Were you thinking of some sort of tighter integration?

All I really want is to ensure the set of objects and APIs available to solve problem X is the minimum spanning set.  If I want to be called back in 100 ms, I should have one API that does that, or a small number that have unavoidably-distinct use cases.  Right now I feel kind of like I could use practically any of our callback APIs to solve most problems.  Maybe my feeling is wrong.

PK 

[William Chan (陈智昌)]

Thanks for sending this out!  I'm really excited to get curried functions!

The one section I'd like to hear opinions on is the ref counting support in the callbacks.  My opinion is primarily derived from net/ which is mostly single-threaded, so I have to admit that I probably have a very narrow perspective on the issue.  I'd love to see counter-examples to the points I'm about to raise.

I have to say that I feel like ref counting in callbacks is generally undesirable.  I feel like people often don't think about, if the last ref is the ref added by RunnableMethodTraits, do we really want to keep the object alive?  I'm suspicious that this is usually the case (if you have good counter-examples here, please list them!).  For the single-threaded case, I think that ScopedRunnableMethodFactory is almost always what you want when posting a task, even if the object is RefCounted.  In the multithreaded case, I think that people unfortunately make their object RefCountedThreadSafe in order to make sure they're alive when the callback is invoked.  However, they wouldn't mind if the object were deleted and the callback not invoked.  There's no good ScopedRunnableMethodFactory for the threadsafe case, which I suspect is why a lot of the objects become RefCountedThreadSafe.  I'm not entirely sure if my conjecture is true, but that's my suspicion.  I feel like refcounting in this case often confuses object ownership and lifetime.  I think a potentially better solution would be to use threadsafe weak pointers (might need to fix base::WeakPtr since it's thread-unsafe).

--

[Andrew Scherkus]

On Mon, Oct 4, 2010 at 8:56 PM, William Chan (陈智昌) <will...@chromium.org> wrote:

Thanks for sending this out!  I'm really excited to get curried functions!

The one section I'd like to hear opinions on is the ref counting support in the callbacks.  My opinion is primarily derived from net/ which is mostly single-threaded, so I have to admit that I probably have a very narrow perspective on the issue.  I'd love to see counter-examples to the points I'm about to raise.

I have to say that I feel like ref counting in callbacks is generally undesirable.  I feel like people often don't think about, if the last ref is the ref added by RunnableMethodTraits, do we really want to keep the object alive?  I'm suspicious that this is usually the case (if you have good counter-examples here, please list them!).  For the single-threaded case, I think that ScopedRunnableMethodFactory is almost always what you want when posting a task, even if the object is RefCounted.  In the multithreaded case, I think that people unfortunately make their object RefCountedThreadSafe in order to make sure they're alive when the callback is invoked.  However, they wouldn't mind if the object were deleted and the callback not invoked.  There's no good ScopedRunnableMethodFactory for the threadsafe case, which I suspect is why a lot of the objects become RefCountedThreadSafe.  I'm not entirely sure if my conjecture is true, but that's my suspicion.  I feel like refcounting in this case often confuses object ownership and lifetime.  I think a potentially better solution would be to use threadsafe weak pointers (might need to fix base::WeakPtr since it's thread-unsafe).

This is sort of how the media code evolved.  Despite having explicit lifetimes it was quite easy to add ref-counting and thread-safeness more or less because NewRunnableMethod() demanded it (and I didn't know about RunnableMethodTraits at the time).

I'm not totally comfortable not knowing which threads gets to delete a running media instance since it's pretty heavyweight and probably janks up whatever thread was unlucky enough to deref the last ref :(

I believe most of the reference counting came from the various BrowserThreads and requiring that the work be executed even if things start shutting down (I recall darin/brettw having some good examples here...)

Andrew

[John Abd-El-Malek]

On Mon, Oct 4, 2010 at 8:56 PM, William Chan (陈智昌) <will...@chromium.org> wrote:

Thanks for sending this out!  I'm really excited to get curried functions!

The one section I'd like to hear opinions on is the ref counting support in the callbacks.  My opinion is primarily derived from net/ which is mostly single-threaded, so I have to admit that I probably have a very narrow perspective on the issue.  I'd love to see counter-examples to the points I'm about to raise.

I have to say that I feel like ref counting in callbacks is generally undesirable.  I feel like people often don't think about, if the last ref is the ref added by RunnableMethodTraits, do we really want to keep the object alive?  I'm suspicious that this is usually the case (if you have good counter-examples here, please list them!).  For the single-threaded case, I think that ScopedRunnableMethodFactory is almost always what you want when posting a task, even if the object is RefCounted.  In the multithreaded case, I think that people unfortunately make their object RefCountedThreadSafe in order to make sure they're alive when the callback is invoked.  However, they wouldn't mind if the object were deleted and the callback not invoked.  

I think both scenarios exist, and so some objects will always want the callback to be run.  However, as you point out, if you don't care about a callback not running if the object is deleted, currently the code only supports the single-threaded case (using ScopedRunnableMethodFactory).  There's no clean solution to when the callback goes to a different thread.  I think this is easy to fix, I'll take a stab at it real soon.

[William Chan (陈智昌)]

On Tue, Oct 5, 2010 at 11:41 AM, John Abd-El-Malek <j...@chromium.org> wrote:

On Mon, Oct 4, 2010 at 8:56 PM, William Chan (陈智昌) <will...@chromium.org> wrote:

Thanks for sending this out!  I'm really excited to get curried functions!

The one section I'd like to hear opinions on is the ref counting support in the callbacks.  My opinion is primarily derived from net/ which is mostly single-threaded, so I have to admit that I probably have a very narrow perspective on the issue.  I'd love to see counter-examples to the points I'm about to raise.

I have to say that I feel like ref counting in callbacks is generally undesirable.  I feel like people often don't think about, if the last ref is the ref added by RunnableMethodTraits, do we really want to keep the object alive?  I'm suspicious that this is usually the case (if you have good counter-examples here, please list them!).  For the single-threaded case, I think that ScopedRunnableMethodFactory is almost always what you want when posting a task, even if the object is RefCounted.  In the multithreaded case, I think that people unfortunately make their object RefCountedThreadSafe in order to make sure they're alive when the callback is invoked.  However, they wouldn't mind if the object were deleted and the callback not invoked.  

I think both scenarios exist, and so some objects will always want the callback to be run.  However, as you point out, if you don't care about a callback not running if the object is deleted, currently the code only supports the single-threaded case (using ScopedRunnableMethodFactory).  There's no clean solution to when the callback goes to a different thread.  I think this is easy to fix, I'll take a stab at it real soon.

You know more about these use cases than I do, so I definitely trust you when you say such scenarios exist.  For my own clarification, can you point out some classes where we need the callback always to be run?

I think that if we can minimize these scenarios, then I'll be happy.  Once we have a good alternative for the multithreaded case and convert the majority of existing cases to using that, then I think that people will hopefully follow the examples of the code around them and will use the alternative.

I looked at the document again, and I noticed that IsRefCounted() is a virtual member function on Closure.  Is that necessary?  I would have thought that we could use type traits for the RefClosure subclass in ChromeThread::PostTask to enforce this without needing the virtual.  The virtual seems to imply that the MessageLoop will be doing the refcounting.  I would have thought that the RefClosure itself would do the refcounting.  Can you clarify this?

[Albert J. Wong (王重傑)]

On Tue, Oct 5, 2010 at 1:30 PM, William Chan (陈智昌) <will...@chromium.org> wrote:

On Tue, Oct 5, 2010 at 11:41 AM, John Abd-El-Malek <j...@chromium.org> wrote:

On Mon, Oct 4, 2010 at 8:56 PM, William Chan (陈智昌) <will...@chromium.org> wrote:

Thanks for sending this out!  I'm really excited to get curried functions!

The one section I'd like to hear opinions on is the ref counting support in the callbacks.  My opinion is primarily derived from net/ which is mostly single-threaded, so I have to admit that I probably have a very narrow perspective on the issue.  I'd love to see counter-examples to the points I'm about to raise.

I have to say that I feel like ref counting in callbacks is generally undesirable.  I feel like people often don't think about, if the last ref is the ref added by RunnableMethodTraits, do we really want to keep the object alive?  I'm suspicious that this is usually the case (if you have good counter-examples here, please list them!).  For the single-threaded case, I think that ScopedRunnableMethodFactory is almost always what you want when posting a task, even if the object is RefCounted.  In the multithreaded case, I think that people unfortunately make their object RefCountedThreadSafe in order to make sure they're alive when the callback is invoked.  However, they wouldn't mind if the object were deleted and the callback not invoked.  

I think both scenarios exist, and so some objects will always want the callback to be run.  However, as you point out, if you don't care about a callback not running if the object is deleted, currently the code only supports the single-threaded case (using ScopedRunnableMethodFactory).  There's no clean solution to when the callback goes to a different thread.  I think this is easy to fix, I'll take a stab at it real soon.

You know more about these use cases than I do, so I definitely trust you when you say such scenarios exist.  For my own clarification, can you point out some classes where we need the callback always to be run?

I think that if we can minimize these scenarios, then I'll be happy.  Once we have a good alternative for the multithreaded case and convert the majority of existing cases to using that, then I think that people will hopefully follow the examples of the code around them and will use the alternative.

I looked at the document again, and I noticed that IsRefCounted() is a virtual member function on Closure.  Is that necessary?  I would have thought that we could use type traits for the RefClosure subclass in ChromeThread::PostTask to enforce this without needing the virtual.  The virtual seems to imply that the MessageLoop will be doing the refcounting.  I would have thought that the RefClosure itself would do the refcounting.  Can you clarify this?

This is not for deciding if something will do refcounting, but rather just to facilitate DCHECKs.  I thought it'd be nice to query the objects for this.

BTW, did people have an opinion on the naming of the factory methods?

-Albert

[John Abd-El-Malek]

On Tue, Oct 5, 2010 at 1:30 PM, William Chan (陈智昌) <will...@chromium.org> wrote:

On Tue, Oct 5, 2010 at 11:41 AM, John Abd-El-Malek <j...@chromium.org> wrote:

On Mon, Oct 4, 2010 at 8:56 PM, William Chan (陈智昌) <will...@chromium.org> wrote:

Thanks for sending this out!  I'm really excited to get curried functions!

The one section I'd like to hear opinions on is the ref counting support in the callbacks.  My opinion is primarily derived from net/ which is mostly single-threaded, so I have to admit that I probably have a very narrow perspective on the issue.  I'd love to see counter-examples to the points I'm about to raise.

I have to say that I feel like ref counting in callbacks is generally undesirable.  I feel like people often don't think about, if the last ref is the ref added by RunnableMethodTraits, do we really want to keep the object alive?  I'm suspicious that this is usually the case (if you have good counter-examples here, please list them!).  For the single-threaded case, I think that ScopedRunnableMethodFactory is almost always what you want when posting a task, even if the object is RefCounted.  In the multithreaded case, I think that people unfortunately make their object RefCountedThreadSafe in order to make sure they're alive when the callback is invoked.  However, they wouldn't mind if the object were deleted and the callback not invoked.  

I think both scenarios exist, and so some objects will always want the callback to be run.  However, as you point out, if you don't care about a callback not running if the object is deleted, currently the code only supports the single-threaded case (using ScopedRunnableMethodFactory).  There's no clean solution to when the callback goes to a different thread.  I think this is easy to fix, I'll take a stab at it real soon.

You know more about these use cases than I do, so I definitely trust you when you say such scenarios exist.  For my own clarification, can you point out some classes where we need the callback always to be run?

Off the top of my head, in all the places of ResourceMessageFilter where we use the IPC_MESSAGE_HANDLER_DELAY_REPLY.  We go off to a different thread to do some work, then post a message back to send the reply to the synchronous message.  Even though it's shutdown when RMF is going away, if the reply is not thread, the renderer will be hung and it won't have a clean shutdown.  It might even happen without shutdown, say if the process is closed while it has an outstanding synchronou XHR.  Then we'd have a zombie renderer.

[William Chan (陈智昌)]

On Tue, Oct 5, 2010 at 2:14 PM, Albert J. Wong (王重傑) <ajw...@chromium.org> wrote:

On Tue, Oct 5, 2010 at 1:30 PM, William Chan (陈智昌) <will...@chromium.org> wrote:

On Tue, Oct 5, 2010 at 11:41 AM, John Abd-El-Malek <j...@chromium.org> wrote:

On Mon, Oct 4, 2010 at 8:56 PM, William Chan (陈智昌) <will...@chromium.org> wrote:

Thanks for sending this out!  I'm really excited to get curried functions!

The one section I'd like to hear opinions on is the ref counting support in the callbacks.  My opinion is primarily derived from net/ which is mostly single-threaded, so I have to admit that I probably have a very narrow perspective on the issue.  I'd love to see counter-examples to the points I'm about to raise.

I have to say that I feel like ref counting in callbacks is generally undesirable.  I feel like people often don't think about, if the last ref is the ref added by RunnableMethodTraits, do we really want to keep the object alive?  I'm suspicious that this is usually the case (if you have good counter-examples here, please list them!).  For the single-threaded case, I think that ScopedRunnableMethodFactory is almost always what you want when posting a task, even if the object is RefCounted.  In the multithreaded case, I think that people unfortunately make their object RefCountedThreadSafe in order to make sure they're alive when the callback is invoked.  However, they wouldn't mind if the object were deleted and the callback not invoked.  

I think both scenarios exist, and so some objects will always want the callback to be run.  However, as you point out, if you don't care about a callback not running if the object is deleted, currently the code only supports the single-threaded case (using ScopedRunnableMethodFactory).  There's no clean solution to when the callback goes to a different thread.  I think this is easy to fix, I'll take a stab at it real soon.

You know more about these use cases than I do, so I definitely trust you when you say such scenarios exist.  For my own clarification, can you point out some classes where we need the callback always to be run?

I think that if we can minimize these scenarios, then I'll be happy.  Once we have a good alternative for the multithreaded case and convert the majority of existing cases to using that, then I think that people will hopefully follow the examples of the code around them and will use the alternative.

I looked at the document again, and I noticed that IsRefCounted() is a virtual member function on Closure.  Is that necessary?  I would have thought that we could use type traits for the RefClosure subclass in ChromeThread::PostTask to enforce this without needing the virtual.  The virtual seems to imply that the MessageLoop will be doing the refcounting.  I would have thought that the RefClosure itself would do the refcounting.  Can you clarify this?

This is not for deciding if something will do refcounting, but rather just to facilitate DCHECKs.  I thought it'd be nice to query the objects for this.

Where are you planning on querying the objects for this?  And don't you think that compile-time checks are better than run time assertions?  if RefClosure is a subclass of Closure, I think we can implement compile-time checks.  That is, unless we want to try to optimize build times.  If so, we'd probably have a closure.h that MessageLoop would include.  Then the callback specializations would be in a generated header file, callback-specializations.h or something.  Only the calls of NewCallback and friends would need this, so hopefully these template definitions wouldn't be required in headers, just individual .cc files, so they won't have to be included everywhere. But if we kept the current way where message_loop.h includes task.h which includes all the specializations, then I think PostClosure should just COMPILE_ASSERT on different type traits for Closure and RefClosure.

 

BTW, did people have an opinion on the naming of the factory methods?

I'm biased here, but I rather like the Google style and would prefer to be consistent with it.

I re-read and noticed MessageLoop seems to be keeping both PostTask() and PostClosure().  Why?  The comments say it's because you can enforce that PostTask gets a RefClosure.  What does that give us?  Is it purely for when reading code, so you know that it's a RefClosure?  I would think that in the vast majority of cases, the NewRefCallback() is called when doing the PostClosure(), so this doesn't gain us anything.

[William Chan (陈智昌)]

On Tue, Oct 5, 2010 at 3:08 PM, John Abd-El-Malek <j...@chromium.org> wrote:

On Tue, Oct 5, 2010 at 1:30 PM, William Chan (陈智昌) <will...@chromium.org> wrote:

On Tue, Oct 5, 2010 at 11:41 AM, John Abd-El-Malek <j...@chromium.org> wrote:

On Mon, Oct 4, 2010 at 8:56 PM, William Chan (陈智昌) <will...@chromium.org> wrote:

Thanks for sending this out!  I'm really excited to get curried functions!

The one section I'd like to hear opinions on is the ref counting support in the callbacks.  My opinion is primarily derived from net/ which is mostly single-threaded, so I have to admit that I probably have a very narrow perspective on the issue.  I'd love to see counter-examples to the points I'm about to raise.

I have to say that I feel like ref counting in callbacks is generally undesirable.  I feel like people often don't think about, if the last ref is the ref added by RunnableMethodTraits, do we really want to keep the object alive?  I'm suspicious that this is usually the case (if you have good counter-examples here, please list them!).  For the single-threaded case, I think that ScopedRunnableMethodFactory is almost always what you want when posting a task, even if the object is RefCounted.  In the multithreaded case, I think that people unfortunately make their object RefCountedThreadSafe in order to make sure they're alive when the callback is invoked.  However, they wouldn't mind if the object were deleted and the callback not invoked.  

I think both scenarios exist, and so some objects will always want the callback to be run.  However, as you point out, if you don't care about a callback not running if the object is deleted, currently the code only supports the single-threaded case (using ScopedRunnableMethodFactory).  There's no clean solution to when the callback goes to a different thread.  I think this is easy to fix, I'll take a stab at it real soon.

You know more about these use cases than I do, so I definitely trust you when you say such scenarios exist.  For my own clarification, can you point out some classes where we need the callback always to be run?

Off the top of my head, in all the places of ResourceMessageFilter where we use the IPC_MESSAGE_HANDLER_DELAY_REPLY.  We go off to a different thread to do some work, then post a message back to send the reply to the synchronous message.  Even though it's shutdown when RMF is going away, if the reply is not thread, the renderer will be hung and it won't have a clean shutdown.  It might even happen without shutdown, say if the process is closed while it has an outstanding synchronou XHR.  Then we'd have a zombie renderer.

I think this is a fine example.  John, in your experience, do you think this is the common case for RefCountedThreadSafe objects?  My hunch is that it isn't.  I feel like more would rather that the default for RunnableMethodTraits not keep their lifetimes across posted tasks.  Maybe all those cases will go away once you fix ScopedRunnableMethodFactory to be thread-safe.  Btw, how does this work?  Will you have a lock?  Does that mean that if thread A posts a ScopedRunnableMethodFactory::NewRunnableMethod to thread B, then thread A could potentially block until thread B finishes running the task (assuming thread B already started running the task before the factory tried to revoke it)?

I'm almost tempted to push for RefClosure to be a second-class citizen in the new callback system.  Perhaps its definition should be in a separate file, and MessageLoop should have no special awareness of RefClosure.  Indeed, Task is not aware of RunnableMethodTraits, that's just an implementation detail of NewRunnableMethod.  With Closures, I sort of think that we should rip the ref counting out of it and make it a set of utility template functions, but not part of the default.

[Darin Fisher]

I really appreciate you guys working on this issue.  I agree that things could be much better.

Some comments:

I'm not sure about the approach of adding PostClosure to MessageLoop.  It seems unnecessary.  Ultimately, MessageLoop is just about scheduling work, and that work is expressed via the Task interface.  The distinction between PostTask and PostClosure seems like it should be expressed at a higher level.

NewCallback, NewPermanentCallback and so on all seem to leave the memory management of the object unspecified.  This seems like something to be avoided, or at least there should be some kind of more explicit opt-in for not having memory management.

In contrast, NewRunnableMethod, ScopedRunnableMethodFactory, ScopedCallbackFactory, and the Timer classs all provide fairly error-proof systems for managing memory.  The latter set all build off of the WeakPtr class, which admittedly means they are designed for single threaded use cases, but the result is that they are super easy to use correctly.  NewRunnableMethod defaults to reference counting the object so that it can work safely.

Perhaps what we need are better tools for cross thread tasks/closures and callbacks in cases where the subject of the method invocation does not want to be reference counted.  RunnableMethodTraits is a fairly sucky solution.  I'm not sure how to use WeakPtr directly since it would seem to require using a Lock, and I believe you'd need to hold the Lock while executing the closure/callback, and that would be bad since it could lead to dead-locks.

As for eliminating friction between Task and Callbacks, it seems to me that we should make it easier to take a CallbackN from a ScopedCallbackFactory and bind the parameters.  The result of that operation should be a Task.  It would also be good to merge Task and Callback0 since there does not really need to be a distinction.  (However, merging those may not be necessary to address most of our concerns.)

It may be tempting to assume that if only we had the google-style callback system that everything would be perfect.  I'm skeptical of that.  I don't think we want to end up in a place where people are frequently using a form of NewCallback that leaves the object unmanaged.  They should instead be using ScopedCallbackFactory::NewCallback or some other tool, which provides memory management by default.

Let's review the high priority problems we would like to fix:

1) Task and Callback0 are really similar.  Why are they different?

2) Why can't I post a CallbackN to another thread?

3) Why force all invokers of a Task/Callback to delete the Task/Callback?  Why not have the Task/Callback self-delete?  If I had this, then I could support "permanent" callbacks.

4) Why do I have to make my class RefCountedThreadSafe when I want to call a method on another thread?

How about a solution like this:

-  Provide a Bind function that given (CallbackRunner*, TupleN) spits out a Task.  Allows us to kill NewRunnableMethod, replacing it with Bind(NewCallback(...), params).

-  Eliminate ScopedRunnableMethodFactory in favor of ScopedCallbackFactory.  Made possible by the Bind method.

-  Make deletion of Tasks and Callbacks the responsibility of the Run method.  This will be a *very* disruptive change because of the hand implemented Tasks.  It may also interfere with the tracked objects system (about:objects), which relies on Task destruction as the done signal.

Now the interesting part is dealing with problem #4.

There are plenty of cases where you want a variant of NewCallback that AddRefs the given object.  That way the callback retains an owning reference to the object it intends to invoke a method on later, and so we don't need to worry about memory management.  (Well, at most we may wish to specify a Traits to RefCountedThreadSafe so that we can designate a thread on which the object should be deleted.)

However, let's consider the case of trying to invoke a method on a singleton object or some object that is known to out-live the target MessageLoop.  In that case, which I would argue is the less common case in Chromium (but maybe very common in src/media/?), perhaps we can come up with an alternative name for NewCallback or some kind of helpful syntax.  Hmm... here's an idea:

  CallbackN<...>::Type* callback = NewCallback(Unretained(obj), &MyClass::Method, param_a, param_b);

Here, the Unretained function is used to request a NewCallback template that will not AddRef obj.  In this way, the memory management (or lack thereof) is explicit and opt-in at the place where we construct the callback.  We could implement Unretained today and use it to delete all of the RunnableMethodTraits crap!

(I would also be very grateful if we could find a way to simplify the syntax for declaring a callback.  CallbackN<...>::Type is pretty verbose!)

Related to the above proposal, I think it would be convenient to add helpers on MessageLoop, MessageLoopProxy and ChromeThread for posting a callback.

  template <...>

  void PostCallback(Location, CallbackN<...>::Type* callback, ...callback args...);

This would be implemented as PostTask(location, Bind(callback, ...callback args...));

Note: When discussing the Bind function, I've ignored the partial bind case.  I am interested in the partial bind case, but I'm not sure how much we need it.  I've seen some cases where it would certainly be useful in the code, but I'd be willing to postpone adding support for it.

So, anyways, that's my thinking on this subject.  Please consider this alternative proposal.

-Darin

On Mon, Oct 4, 2010 at 2:58 PM, Albert J. Wong (王重傑) <ajw...@chromium.org> wrote:

--

[John Abd-El-Malek]

On Tue, Oct 5, 2010 at 10:21 PM, William Chan (陈智昌) <will...@chromium.org> wrote:

On Tue, Oct 5, 2010 at 3:08 PM, John Abd-El-Malek <j...@chromium.org> wrote:

On Tue, Oct 5, 2010 at 1:30 PM, William Chan (陈智昌) <will...@chromium.org> wrote:

On Tue, Oct 5, 2010 at 11:41 AM, John Abd-El-Malek <j...@chromium.org> wrote:

On Mon, Oct 4, 2010 at 8:56 PM, William Chan (陈智昌) <will...@chromium.org> wrote:

Thanks for sending this out!  I'm really excited to get curried functions!

The one section I'd like to hear opinions on is the ref counting support in the callbacks.  My opinion is primarily derived from net/ which is mostly single-threaded, so I have to admit that I probably have a very narrow perspective on the issue.  I'd love to see counter-examples to the points I'm about to raise.

I have to say that I feel like ref counting in callbacks is generally undesirable.  I feel like people often don't think about, if the last ref is the ref added by RunnableMethodTraits, do we really want to keep the object alive?  I'm suspicious that this is usually the case (if you have good counter-examples here, please list them!).  For the single-threaded case, I think that ScopedRunnableMethodFactory is almost always what you want when posting a task, even if the object is RefCounted.  In the multithreaded case, I think that people unfortunately make their object RefCountedThreadSafe in order to make sure they're alive when the callback is invoked.  However, they wouldn't mind if the object were deleted and the callback not invoked.  

I think both scenarios exist, and so some objects will always want the callback to be run.  However, as you point out, if you don't care about a callback not running if the object is deleted, currently the code only supports the single-threaded case (using ScopedRunnableMethodFactory).  There's no clean solution to when the callback goes to a different thread.  I think this is easy to fix, I'll take a stab at it real soon.

You know more about these use cases than I do, so I definitely trust you when you say such scenarios exist.  For my own clarification, can you point out some classes where we need the callback always to be run?

Off the top of my head, in all the places of ResourceMessageFilter where we use the IPC_MESSAGE_HANDLER_DELAY_REPLY.  We go off to a different thread to do some work, then post a message back to send the reply to the synchronous message.  Even though it's shutdown when RMF is going away, if the reply is not thread, the renderer will be hung and it won't have a clean shutdown.  It might even happen without shutdown, say if the process is closed while it has an outstanding synchronou XHR.  Then we'd have a zombie renderer.

I think this is a fine example.  John, in your experience, do you think this is the common case for RefCountedThreadSafe objects?  My hunch is that it isn't.  I feel like more would rather that the default for RunnableMethodTraits not keep their lifetimes across posted tasks.  Maybe all those cases will go away once you fix ScopedRunnableMethodFactory to be thread-safe.  Btw, how does this work?  Will you have a lock?  Does that mean that if thread A posts a ScopedRunnableMethodFactory::NewRunnableMethod to thread B, then thread A could potentially block until thread B finishes running the task (assuming thread B already started running the task before the factory tried to revoke it)?

Definitely we would want to avoid one thread blocking on another, since that'd lead to deadlocks.  I was thinking that perhaps there would be a way to post task to a static member function, along with passing an extra parameter that allows one to run a method on the original thread.  This would solve a common use case of going to the file thread to do a slow operation, and then wanting to get back to the original thread.  Even with objects that currently derive from RefCountedThreadSafe, often many of the member variables are not thread safe, and the user passes the needed information using parameters.  The above would get rid of a bunch of hacky code I've personally written :)

[Albert J. Wong (王重傑)]

Hey Darin,

Sorry for taking so long to respond. Been pondering what you wrote.

First off, I want to cut apart the "google-style callback" system from how the callback system enforces memory management (if any) on its target objects.  To me, the former is merely a set of types for representing functions, and a set of factories that do the binding.  The latter is an attribute that can be added on to the behavior of almost any such system.

For the question of callbacks + memory management, I think Will brings up an interesting (and somewhat touchy) topic of of whether we should try to reduce the reliance on shared ownership by the callback...possibly by defaulting to no refcounts. If people are interested in discussing that, we should fork this thread?  The only effect it should have on this API is what the defaults are.

Back to the specific API of the callback system.

I really like your idea of wrapping the object to specify the behavior during NewCallback:

    NewCallback(Unretained(&obj), &MyClass::Method, param_a));

rather than having 2 sets of factory methods. 1/2s the complexity.  If the code can be made to work out, this seems like a really clean solution.

About using a "Bind" function that takes a CallbackN object as the first parameter and a set of arguments to prebind, that would work. However, it seems like we're costing an extra set of functions, and one more object instantiation, when we might as well just make NewCallback do that work. Is there something I'm missing here?

For self-deleting callbacks (where cb->Run() is responsible for deleting itself), I wasn't clear on what your stance was.  It is definitely a very disruptive change.  We could make the initial transition relatively easy by doing an s/NewCallback/NewPermanentCallback/ throughout the codebase. I don't understand what the issue is with object tracking.  All the callbacks are still getting deleted roughly where they would be anyways aren't they?

As for the implicit suggestion to evolved the existing Callback code rather than replace it wholesale, the reasons I was leaning towards a "rewrite then migrate" were:

   a) We could stage the transition easier, and vet the new API for a little bit first.

   b) I was expecting a non-tuple based implementation to be more memory efficient (less template instantiations), which would mean it'd be a full rewrite anyways.

Either method is completely doable.  But it felt safer to redo.  Especially if (b) is true (I still need to measure it), then the "evolution" would end up becoming a rewrite anyways.

So to summarize:

  1) Keep the set of NewCallback factories, and don't use a Bind() function.

  2) Don't have both NewCallback and NewRefCallback.  Choose one default (probably refcounted unless people say otherwise).

  3) Use the Unretained() object wrapper to cut the refcounting relation.

  4) Didn't fully understand your stance on self-deleting callbacks.

  5) For making the changes, I'm still inclined to reimplement/migrate for the reasons above.

Thoughts?

-Albert

[William Chan (陈智昌)]

On Wed, Oct 6, 2010 at 9:28 AM, John Abd-El-Malek <j...@chromium.org> wrote:

On Tue, Oct 5, 2010 at 10:21 PM, William Chan (陈智昌) <will...@chromium.org> wrote:

On Tue, Oct 5, 2010 at 3:08 PM, John Abd-El-Malek <j...@chromium.org> wrote:

On Tue, Oct 5, 2010 at 1:30 PM, William Chan (陈智昌) <will...@chromium.org> wrote:

On Tue, Oct 5, 2010 at 11:41 AM, John Abd-El-Malek <j...@chromium.org> wrote:

On Mon, Oct 4, 2010 at 8:56 PM, William Chan (陈智昌) <will...@chromium.org> wrote:

Thanks for sending this out!  I'm really excited to get curried functions!

The one section I'd like to hear opinions on is the ref counting support in the callbacks.  My opinion is primarily derived from net/ which is mostly single-threaded, so I have to admit that I probably have a very narrow perspective on the issue.  I'd love to see counter-examples to the points I'm about to raise.

I have to say that I feel like ref counting in callbacks is generally undesirable.  I feel like people often don't think about, if the last ref is the ref added by RunnableMethodTraits, do we really want to keep the object alive?  I'm suspicious that this is usually the case (if you have good counter-examples here, please list them!).  For the single-threaded case, I think that ScopedRunnableMethodFactory is almost always what you want when posting a task, even if the object is RefCounted.  In the multithreaded case, I think that people unfortunately make their object RefCountedThreadSafe in order to make sure they're alive when the callback is invoked.  However, they wouldn't mind if the object were deleted and the callback not invoked.  

I think both scenarios exist, and so some objects will always want the callback to be run.  However, as you point out, if you don't care about a callback not running if the object is deleted, currently the code only supports the single-threaded case (using ScopedRunnableMethodFactory).  There's no clean solution to when the callback goes to a different thread.  I think this is easy to fix, I'll take a stab at it real soon.

You know more about these use cases than I do, so I definitely trust you when you say such scenarios exist.  For my own clarification, can you point out some classes where we need the callback always to be run?

Off the top of my head, in all the places of ResourceMessageFilter where we use the IPC_MESSAGE_HANDLER_DELAY_REPLY.  We go off to a different thread to do some work, then post a message back to send the reply to the synchronous message.  Even though it's shutdown when RMF is going away, if the reply is not thread, the renderer will be hung and it won't have a clean shutdown.  It might even happen without shutdown, say if the process is closed while it has an outstanding synchronou XHR.  Then we'd have a zombie renderer.

I think this is a fine example.  John, in your experience, do you think this is the common case for RefCountedThreadSafe objects?  My hunch is that it isn't.  I feel like more would rather that the default for RunnableMethodTraits not keep their lifetimes across posted tasks.  Maybe all those cases will go away once you fix ScopedRunnableMethodFactory to be thread-safe.  Btw, how does this work?  Will you have a lock?  Does that mean that if thread A posts a ScopedRunnableMethodFactory::NewRunnableMethod to thread B, then thread A could potentially block until thread B finishes running the task (assuming thread B already started running the task before the factory tried to revoke it)?

Definitely we would want to avoid one thread blocking on another, since that'd lead to deadlocks.  I was thinking that perhaps there would be a way to post task to a static member function, along with passing an extra parameter that allows one to run a method on the original thread.  This would solve a common use case of going to the file thread to do a slow operation, and then wanting to get back to the original thread.  Even with objects that currently derive from RefCountedThreadSafe, often many of the member variables are not thread safe, and the user passes the needed information using parameters.  The above would get rid of a bunch of hacky code I've personally written :)

Doh, I should have read your email before tonight, so I could have talked to you about this today in person.  I fail.  Anyway, I think that for this case, you can probably use base::WeakPtr.  Construct a Task that keeps a base::WeakPtr<T>.  In Run(), it checks ptr_.get() to make sure it's non-NULL, and if so, invokes a function pointer with the params.  You can imagine a NewWeakRunnableMethod() implementation that does this.  Take this task and pass it as a parameter to the task you're posting to the static member function.  NewWeakRunnableMethod()'s constructed Task can inherit from NonThreadSafe to help enforce safety.

I don't think there's a way to get rid of refcounting for the general case of posting tasks to another thread.  Ideally what I'd want is a theadsafe weak pointer that I could call lock() on to create a scoped_refptr that is NULL if it's already deleted or non-NULL if it's still alive.  Again, this should be wrapped by some helper function that does the lock() and NULL check to decide where or not to invoke the runnable method.

[John Abd-El-Malek]

On Thu, Oct 7, 2010 at 10:48 PM, William Chan (陈智昌) <will...@chromium.org> wrote:

On Wed, Oct 6, 2010 at 9:28 AM, John Abd-El-Malek <j...@chromium.org> wrote:

On Tue, Oct 5, 2010 at 10:21 PM, William Chan (陈智昌) <will...@chromium.org> wrote:

On Tue, Oct 5, 2010 at 3:08 PM, John Abd-El-Malek <j...@chromium.org> wrote:

On Tue, Oct 5, 2010 at 1:30 PM, William Chan (陈智昌) <will...@chromium.org> wrote:

On Tue, Oct 5, 2010 at 11:41 AM, John Abd-El-Malek <j...@chromium.org> wrote:

On Mon, Oct 4, 2010 at 8:56 PM, William Chan (陈智昌) <will...@chromium.org> wrote:

Thanks for sending this out!  I'm really excited to get curried functions!

The one section I'd like to hear opinions on is the ref counting support in the callbacks.  My opinion is primarily derived from net/ which is mostly single-threaded, so I have to admit that I probably have a very narrow perspective on the issue.  I'd love to see counter-examples to the points I'm about to raise.

I have to say that I feel like ref counting in callbacks is generally undesirable.  I feel like people often don't think about, if the last ref is the ref added by RunnableMethodTraits, do we really want to keep the object alive?  I'm suspicious that this is usually the case (if you have good counter-examples here, please list them!).  For the single-threaded case, I think that ScopedRunnableMethodFactory is almost always what you want when posting a task, even if the object is RefCounted.  In the multithreaded case, I think that people unfortunately make their object RefCountedThreadSafe in order to make sure they're alive when the callback is invoked.  However, they wouldn't mind if the object were deleted and the callback not invoked.  

I think both scenarios exist, and so some objects will always want the callback to be run.  However, as you point out, if you don't care about a callback not running if the object is deleted, currently the code only supports the single-threaded case (using ScopedRunnableMethodFactory).  There's no clean solution to when the callback goes to a different thread.  I think this is easy to fix, I'll take a stab at it real soon.

You know more about these use cases than I do, so I definitely trust you when you say such scenarios exist.  For my own clarification, can you point out some classes where we need the callback always to be run?

Off the top of my head, in all the places of ResourceMessageFilter where we use the IPC_MESSAGE_HANDLER_DELAY_REPLY.  We go off to a different thread to do some work, then post a message back to send the reply to the synchronous message.  Even though it's shutdown when RMF is going away, if the reply is not thread, the renderer will be hung and it won't have a clean shutdown.  It might even happen without shutdown, say if the process is closed while it has an outstanding synchronou XHR.  Then we'd have a zombie renderer.

I think this is a fine example.  John, in your experience, do you think this is the common case for RefCountedThreadSafe objects?  My hunch is that it isn't.  I feel like more would rather that the default for RunnableMethodTraits not keep their lifetimes across posted tasks.  Maybe all those cases will go away once you fix ScopedRunnableMethodFactory to be thread-safe.  Btw, how does this work?  Will you have a lock?  Does that mean that if thread A posts a ScopedRunnableMethodFactory::NewRunnableMethod to thread B, then thread A could potentially block until thread B finishes running the task (assuming thread B already started running the task before the factory tried to revoke it)?

Definitely we would want to avoid one thread blocking on another, since that'd lead to deadlocks.  I was thinking that perhaps there would be a way to post task to a static member function, along with passing an extra parameter that allows one to run a method on the original thread.  This would solve a common use case of going to the file thread to do a slow operation, and then wanting to get back to the original thread.  Even with objects that currently derive from RefCountedThreadSafe, often many of the member variables are not thread safe, and the user passes the needed information using parameters.  The above would get rid of a bunch of hacky code I've personally written :)

Doh, I should have read your email before tonight, so I could have talked to you about this today in person.  I fail.  Anyway, I think that for this case, you can probably use base::WeakPtr.  Construct a Task (1) that keeps a base::WeakPtr<T>.  In Run(), it checks ptr_.get() to make sure it's non-NULL, and if so, invokes a function pointer with the params.  You can imagine a NewWeakRunnableMethod() implementation that does this.  Take this task and pass it as a parameter to the task you're posting to the static member function (2).  NewWeakRunnableMethod()'s constructed Task can inherit from NonThreadSafe to help enforce safety.

How will the static function create a new task though?  i.e. if (1) above is constructed while on the initial thread, it won't have the parameters that the static function wants to post.  i.e. even at (2) we don't have the parameters that the static function wants to post back.

ideally we want some extra parameter to the static function which it can call NewRunnableX(y, parameter1, parameter2)...

[Albert J. Wong (王重傑)]

Okay, back on this thread.

I had a discussion with zhanyong (of gmock fame), and he brought up the point that we could actually consider tr1::function and tr1::bind if we hacked around the RTTI bug in gcc-4.2 on the mac.  This sounds like it would be a really slick way to go forward as it would keep us standards compliant.

Pros:

  1) all function objects are value types.  No need to worry about deleting since they are passed by copy.

  2) handles all the prebinding, reference parameter, etc., for free.

  3) is the up-and-coming standard.

Cons:

  1) It's more invasive since the function object switch kinds (from pointers to 

      value types).  We can still stage transition though.

  2) Requires a hack on Mac to work around the RTTI bug.

Modifications required:

  1) We need a new "Task" equivalent that wraps a tr1::function<void ()>

      with object tracking. This new object would be a value type.

  2) On mac, we would need to fork a copy of the tr1::functional header, and

      fix the accidental RTTI dependency.

If people are okay with this, this sounds like actually a really nice path forward.  It takes more work, but I'm willing to push through it if everyone agrees.

Any objections?  If not, I will work on checking in the the fork of tr1::function in mac, and creating new versions of ScopedRunnableMethodFactory, and the Task wrapper.  Then plot a migration.

-Albert

[Darin Fisher]

Interesting idea.  I think it would be helpful if you showed some examples of before and after.  What will existing code look like after such a conversion?

-Darin

[Albert J. Wong (王重傑)]

Good point about having examples. Here's what I expect the API would look like (mostly coding off the top of my head):

Task and message loop PostTask:

// This class is intentionally copyable.  We need if we want object

// tracking and tracing.  If we don't care, we could just use tr1::function<void()>

class Task2 {

 public:

  // mimic tr1::function<void ()>'s interface.

  void operator() { func_(); }

 private:

  // This is the tr1 function pointer.

  tr1::function<void()> func_;

  // This doesn't exist yet. At it's simplest, this could be

// a shared_ref_ptr<tracked_objects::Tracked>.

  tracked_objects::TrackedHandle tracked_;

};

// Message Loop interface

MessageLoop::PostTask2(Task2 t);  // Notice this is pass by copy.

Simple NewRunnableMethod:

   scoped_refptr<Foo> f = new Foo();

   Task* t1 = NewRunnableMethod(f, &Foo::func, p1, p2); 

   t1->Run();

   delete t1;

   // t2 can be passed by copy to other functions.

   Task2 t2(&Foo::func, *f, p1, p2);

   t2();

Simple NewCallback.  Uses bare tr1::function

   Bar b;

   Callback2<int, double>::type* c1 = NewCallback(&b, &Bar::func);

   c1->Run(1, 2.0); 

   delete c1;

   tr1::function<void (int, double)> c2 = tr1::bind(&Bar::func, b);

   c1(1, 2.0);

Bridging Callback0::type to Task (this syntax doesn't exist currently):

   Task* t1 = NewTaskCallbackAdapter(NewCallback(&b, &Bar::no_arg)));

   t->Run();

   delete t;

   // You'd only do this if up needed to "upconvert" a tr1::function<void()>

   // that you received from an argument or something.

   Task2 t2 = NewTask(my_void_tr1_function_object);

   t2();

Callbacks with results (does not exist currently):

   int multiply(int a, int b) {return a * b; }

   tr1::function<int (int, int)> f = tr1::bind(&multiply);

   cout << f(2, 3);  // outputs 6

Callbacks with reference parameters (Don't think this exists currently)):

  int identity(int& n) { return n; }

  int n = 1;

  tr1::function<int ()> f = tr1::bind(&multiply, n);

  cout << f();  // outputs 1

  n = 2;

  cout << f();  // still outputs 1

  // Now, we bind n as a reference.

  tr1::function<int ()> f = tr1::bind(&multiply, tr1::ref(n));

  cout << f();  // outputs 2

  n = 3;

  cout << f();  // now outputs 3

That should cover the simple cases.  The two other major cases that I think exist are

  a) ScopedRunnableMethodFactory.

  b) custom written Task and CallbackRunner classes.

For (a), I'm pretty sure we could convert the existing, non-thread-safe, code over easily.  With tr1::bind, you could probably even just composite the weak-pointer check on top of the actual function pointer.  I don't think I understand what John and Will are trying to do in terms of making ScopedRunnableMethodFactory more thread friendly to say for certain that we could support it...but I also can't see the tr1-based system being less flexible.

For (b), I'm hoping with the prebinding, refcounting, result type, and partial binding fixes, these can all be removed.

Thoughts?

-Albert