Re: nsThread now leaks runnables if dispatch fails

On Wed, Jan 6, 2016 at 3:53 AM, Kyle Huey wrote: > On Mon, Jan 4, 2016 at 4:10 PM, Randell Jesup wrote: >> >> Yup. In cases where we anticipate a possible Dispatch failure (which is >> supposed to become impossible, but isn't currently) you can use the >>

Re: nsThread now leaks runnables if dispatch fails

Xidorn Quan writes: > On Thu, Jan 7, 2016 at 6:47 AM, Karl Tomlinson wrote: >> Xidorn Quan writes: >> >>> You can keep a raw pointer yourself, and release it manually after you >>> find the dispatch fails, like what is done in >>> NS_DispatchToCurrentThread [1]. It is ugly,

Re: nsThread now leaks runnables if dispatch fails

On Thu, Jan 7, 2016 at 6:47 AM, Karl Tomlinson wrote: > Xidorn Quan writes: > >> On Wed, Jan 6, 2016 at 3:53 AM, Kyle Huey wrote: >>> On Mon, Jan 4, 2016 at 4:10 PM, Randell Jesup wrote: Yup. In cases where we anticipate a

Re: nsThread now leaks runnables if dispatch fails

Xidorn Quan writes: > On Wed, Jan 6, 2016 at 3:53 AM, Kyle Huey wrote: >> On Mon, Jan 4, 2016 at 4:10 PM, Randell Jesup wrote: >>> >>> Yup. In cases where we anticipate a possible Dispatch failure (which is >>> supposed to become impossible, but isn't

Re: nsThread now leaks runnables if dispatch fails

>> In bug 1218297 we saw a case where dispatch to a thread (the socket >> transport service thread in this case) fails because the thread has >> already shut down. In our brave new world, nsThread simply leaks the >> runnable. It can't release the reference it holds, because that would >>

Re: nsThread now leaks runnables if dispatch fails

On Mon, Jan 4, 2016 at 4:10 PM, Randell Jesup wrote: >>Kyle Huey writes: >> >>> (This is a continuation of the discussion in bug 1218297) >>> >>> In bug 1155059 we made nsIEventTarget::Dispatch take an >>> already_AddRefed instead of a raw pointer. This was done to allow the

Re: nsThread now leaks runnables if dispatch fails

The "race condition" here is the question of which thread the runnable destructor runs on. If the executing thread is already shut down the destructor still runs on the "wrong" dispatching thread (or not at all). Since the point of fixing the original race is to guarantee that the destructor

nsThread now leaks runnables if dispatch fails

(This is a continuation of the discussion in bug 1218297) In bug 1155059 we made nsIEventTarget::Dispatch take an already_AddRefed instead of a raw pointer. This was done to allow the dispatcher to transfer its reference to the runnable to the thread the runnable will run on. That solves a race

Re: nsThread now leaks runnables if dispatch fails

Kyle Huey writes: > (This is a continuation of the discussion in bug 1218297) > > In bug 1155059 we made nsIEventTarget::Dispatch take an > already_AddRefed instead of a raw pointer. This was done to allow the > dispatcher to transfer its reference to the runnable to the thread the > runnable

Re: nsThread now leaks runnables if dispatch fails

>Kyle Huey writes: > >> (This is a continuation of the discussion in bug 1218297) >> >> In bug 1155059 we made nsIEventTarget::Dispatch take an >> already_AddRefed instead of a raw pointer. This was done to allow the >> dispatcher to transfer its reference to the runnable to the thread the >>

Re: nsThread now leaks runnables if dispatch fails

It seems like we should make the default behavior infallible + assert, and introduce a separate dispatch method for the fallible cases. On Mon, Jan 4, 2016 at 10:50 AM, Kyle Huey wrote: > (This is a continuation of the discussion in bug 1218297) > > In bug 1155059 we made

Re: nsThread now leaks runnables if dispatch fails

> In bug 1218297 we saw a case where dispatch to a thread (the socket > transport service thread in this case) fails because the thread has > already shut down. In our brave new world, nsThread simply leaks the > runnable. It can't release the reference it holds, because that would > reintroduce