552009 - IPDL: Avoid atomic refcounting in *Channel

Status: NEW

(Core :: IPC, defect)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Bug 545342 covered Task's outliving the channel, but the solution used refcounting and so had to use atomic refcounting.  I think this is adding unnecessary expense; since *Channel code on the IO thread is bound by strong invariants to channel errors and/or Clear(), I think we can utilize these invariants to avoid atomic refcounting.  One way might be to add a RunnableMethodFactory* to AsyncChannel and use it to post all *Channel tasks.  On Clear(), the factory would be deleted, and all lingering tasks in the main queue would end up canceled.

Comment 1

[Josh Matthews [:jdm]]

Ugh.  So it turns out that ScopedRunnableMethodFactory isn't threadsafe, and so can only be used with |Messageloop::current()->PostTask(FROM_HERE, factory->NewRunnableMethod(...))|.  We're posting to the non-current threads, which ends up triggering abort()s in the patch I've cooked up.

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

(In reply to comment #1)
> Ugh.  So it turns out that ScopedRunnableMethodFactory isn't threadsafe

For *Channel, why not?  Remember, we're guaranteeing the threadsafety of *Channel on behalf of the factory: Clear() only happens on the worker thread, and after that, the IO thread is dead wrt *Channel.

> and so
> can only be used with |Messageloop::current()->PostTask(FROM_HERE,
> factory->NewRunnableMethod(...))|.  We're posting to the non-current threads,
> which ends up triggering abort()s in the patch I've cooked up.

Assuming the factory will be threadsafe for *Channel, use mWorkerLoop->PostTask(...).  (PostTask() is threadsafe.)

Comment 3

[Josh Matthews [:jdm]]

Attachment: First stab at RunnableMethod factories

WIP patch