Bug 1695580 Comment 0 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

Consider `delayedRunnable = DelayedRunnable(target, runnable, delay)`. Dispatching it works roughly like this:
- dispatch `delayedRunnable` to `target`
- DelayedRunnable::Run checks whether delay has passed, if not starts a timer `timer` to run `delayedRunnable` on `target` after the total `delay`
- when `timer` fires it [dispatches `delayedRunnable` to `target` from the global TimerThread](https://searchfox.org/mozilla-central/rev/644e42ded761d4f3ce108fa776197730a9a2c535/xpcom/threads/TimerThread.cpp#734)

Alas, if `target` has been shut down by the time the timer fires, [the TimerThread releases the timer](https://searchfox.org/mozilla-central/rev/644e42ded761d4f3ce108fa776197730a9a2c535/xpcom/threads/TimerThread.cpp#422-439). In the case of DelayedRunnable it leaks. DelayedRunnable has a strong-ref to the timer, and the timer has a strong-ref to the DelayedRunnable through its callback member.

This seems [deliberate](https://searchfox.org/mozilla-central/rev/644e42ded761d4f3ce108fa776197730a9a2c535/xpcom/threads/TimerThread.cpp#439); if the callback was released by the timer at this point, it would be released on a different thread than `target`, which could lead to a number of issues.

I think it would be reasonable to cancel any pending timers as part of `target` shutdown, probably as the last thing that happens on `target`.
Consider `delayedRunnable = DelayedRunnable(target, runnable, delay)`. Dispatching it works roughly like this:
- dispatch `delayedRunnable` to `target`, and starts a timer `timer` to run `delayedRunnable` on `target` after `delay`
- DelayedRunnable::Run checks whether delay has passed, if not waits for the timer `timer`
- when `timer` fires it [dispatches `delayedRunnable` to `target` from the global TimerThread](https://searchfox.org/mozilla-central/rev/644e42ded761d4f3ce108fa776197730a9a2c535/xpcom/threads/TimerThread.cpp#734)

Alas, if `target` has been shut down by the time the timer fires, [the TimerThread releases the timer](https://searchfox.org/mozilla-central/rev/644e42ded761d4f3ce108fa776197730a9a2c535/xpcom/threads/TimerThread.cpp#422-439). In the case of DelayedRunnable it leaks. DelayedRunnable has a strong-ref to the timer, and the timer has a strong-ref to the DelayedRunnable through its callback member.

This seems [deliberate](https://searchfox.org/mozilla-central/rev/644e42ded761d4f3ce108fa776197730a9a2c535/xpcom/threads/TimerThread.cpp#439); if the callback was released by the timer at this point, it would be released on a different thread than `target`, which could lead to a number of issues.

I think it would be reasonable to cancel any pending DelayedRunnable timers as part of `target` shutdown, probably as the last thing that happens on `target`.

Back to Bug 1695580 Comment 0