Bug 182365 – [JSCOnly] Ensure RunLoop::Timer is robust to being deleted inside its user callback

• Status: RESOLVED
• Resolution: INVALID
• Component: Web Template Framework

[Michael Catanzaro]

Ensure RunLoop::Timer is robust to being deleted inside its user callback. This is a theoretical issue that I noticed as the result of an actual use-after-free caught by asan in WPE and GTK. See bug #182271. It's not actually possible to test the original reproducer using JSCOnly, because it was a WebKit-layer problem.

I'm going to attach a totally-untested, speculative fix for the theoretical issue. I think it's correct.

[Michael Catanzaro]

Created attachment 332825 [details]
Patch

[Michael Catanzaro]

This should be reviewed by Yusuke, because I'm really not sure whether it's necessary or not. If the RunLoop itself is guaranteed to have another ref on the ScheduledTask, then this isn't needed.

[Yusuke Suzuki]

(In reply to Michael Catanzaro from comment #2)
> This should be reviewed by Yusuke, because I'm really not sure whether it's
> necessary or not. If the RunLoop itself is guaranteed to have another ref on
> the ScheduledTask, then this isn't needed.

I don't think it is necessary. See L173. When calling ScheduledTask::fired(), RunLoop's code always has ref by `RefPtr<ScheduledTask>`.
BTW, this problem is why I separate ScheduledTask from TimerBase IIRC :)

[Michael Catanzaro]

(In reply to Yusuke Suzuki from comment #3)
> I don't think it is necessary. See L173. When calling
> ScheduledTask::fired(), RunLoop's code always has ref by
> `RefPtr<ScheduledTask>`.
> BTW, this problem is why I separate ScheduledTask from TimerBase IIRC :)

Good decision ;)