Bugzilla@Mozilla – Bug 642717
Refresh driver needs to hold strong refs to |targets| in Notify
Last modified: 2011-05-24 23:35:03 PDT
Summon comment box
I came across this while working on bug 607529 (and the patch for that bug includes a fix for this one, but that bug is not landing in fx4). I believe that the current code in Notify is unsafe. |targets| holds weak references, and since they're on the stack they can't be removed when the documents go away. So if one of the events triggers script that ends up collecting some of the other documents, then we could end up doing event dispatch to a dead event target, which seems bad. I'll attach a minimal fix here. I don't think this is worth respinning for, but I do think we should take this ASAP once we start taking .x fixes.
Created attachment 520121 [details] [review] Hold strong references to our MozBeforePaint event targets.
Comment on attachment 520121 [details] [review] Hold strong references to our MozBeforePaint event targets. Requesting approval, in case we want to fix this for mobile even without fixing it for desktop...
we can pick this up whenever it lands for desktop. not holding mobile4.0 for this.
It sounds bad in theory, but do we know if this happens in practice (for example, is there a testcase that demonstrates the problem)?
I haven't tried writing one, but I could do that if you want...
Daniel, see comment 5.
Pushed http://hg.mozilla.org/mozilla-central/rev/ced2b9373c5f
I really think we need to take this for any security-update releases we do for 2.0. Renominating accordingly.
Comment on attachment 520121 [details] [review] Hold strong references to our MozBeforePaint event targets. Approved for the mozilla2.0 repository, a=dveditz for release-drivers
http://hg.mozilla.org/releases/mozilla-2.0/rev/7bf6e15028ed