While looking at bug 1142803 I noticed that Action uses threadsafe ref-counting. This is not strictly correct as we require these objects to survive until they complete on their owning thread. It would be much safer to simply forbid Release()'ing Actions off their owning thread. Changing Action back to non-threadsafe works for most of the Actions without difficulty. The only problematic Action is that async CachePutAllAction. It AddRef()'s on the IO thread while performing stream copying. This is not necessary, though, and can be replaced with non-owning weak refs due to our Action life cycle guarantees.

NS_NewNonOwningRunnableMethodWithArgs() exists in the tree, but is not used. As a result it has a typo and won't compile if you try to use it. This fixes the typo.

Oops. That was the wrong patch.

This makes Action use non-threadsafe ref-counting. It requires CachePutAllAction to depend on our life cycle guarantees so that it can use non-owning references. I think that is safer than possibly allowing a destructor on a separate thread when we must have completion on the owning thread.

Comment on attachment 8577051 [details] [diff] [review] P2 Make Cache Action use non-threadsafe ref-counting. r=ehsan Actually, I'm not sure this is advisable. After sleeping on it I now remember that we need to hold a strong ref in the async action case to prevent the object from being destroyed out from under itself when it calls Resolve().

Comment on attachment 8577049 [details] [diff] [review] P1 Fix NS_NewNonOwningRunnableMethodWithArgs() so that it compiles. r=froyd nj Review of attachment 8577049 [details] [diff] [review]: ----------------------------------------------------------------- Doh.

I think this will be safe if we land the P3 patch from bug 1110487. That ensures that the Action does not get deleted out from under the method that calls Resolve().

Comment on attachment 8577051 [details] [diff] [review] P2 Make Cache Action use non-threadsafe ref-counting. r=ehsan The changes to Context I've proposed in bug 1110487 allow us to tighten the constrains on Action.

I included this one in some larger try builds I did: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0b2ef58ad79e The devtools failures are unrelated to these patches.

remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/07234a94ed48 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/2e9b6048bd0f

https://hg.mozilla.org/mozilla-central/rev/07234a94ed48 https://hg.mozilla.org/mozilla-central/rev/2e9b6048bd0f