Closed
Bug 757663
Opened 12 years ago
Closed 12 years ago
AddonManagerInternal.callAddonlListeners() informs it's listeners unsafely, causing silent skips in the notification loop
Categories
(Toolkit :: Add-ons Manager, defect)
Toolkit
Add-ons Manager
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: xabolcs, Assigned: Unfocused)
Details
(Whiteboard: [fixed-in-fx-team])
Attachments
(1 file, 1 obsolete file)
15.83 KB,
patch
|
darktrojan
:
review+
mossop
:
review+
|
Details | Diff | Splinter Review |
+++ This bug was initially created as a clone of Bug #756960 +++ User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:14.0) Gecko/20120518 Firefox/14.0a2 Build ID: 20120518042008 Steps to reproduce: Part 1: 1. got a Toolkit enabled App with at least one addon 2. open up Error Console 3. add the AddonListeners from the pastebin [1], they will listen for enabling / disabling 4. disable / enable an addon 5. notice the "foo says ...", "bar says ..." and "foobar says ..." messages Part 2: 6. restart App to cleanup 7. open Error Console 8. uncomment the removeAddonListener() related lines in listenerFoo.onDisabling() 9. evaluate this modified code in Error Console 10. disable / enable an addon 11. notice the missing "... disabling!" message from the second, listenerBar listner 12. disable an addon again 13. listenerFoo is now removed, so it doesn't modifies the Array while being in the loop, so all the listeners are notified Actual results: Second listener doesn't fire. Expected results: Second listener should fire. [1] - http://pastebin.mozilla.org/1648327
Reporter | ||
Comment 1•12 years ago
|
||
The STR is for AMI_callAddonListeners, but AMI_callInstallListeners in rare cases could run into unsafe path too.
Summary: AMI_callInstallListeners (in AddonManager.jsm) informs it's listeners unsafely, causing silent skips in the notification loop → AMI_callAddonListeners (in AddonManager.jsm) informs it's listeners unsafely, causing silent skips in the notification loop
Assignee | ||
Comment 2•12 years ago
|
||
Hm, indeed - the array is changed while it's being iterated over. It really needs to take a snapshot of the array, and iterate over that. callManagerListeners has the same issue, as does callInstallListeners when aExtraListeners isn't passed in. I wouldn't expect typeListeners to be a problem, but better to be on the safe side.
Summary: AMI_callAddonListeners (in AddonManager.jsm) informs it's listeners unsafely, causing silent skips in the notification loop → AddonManagerInternal.callAddonlListeners() informs it's listeners unsafely, causing silent skips in the notification loop
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → bmcbride
Status: NEW → ASSIGNED
Reporter | ||
Comment 3•12 years ago
|
||
(In reply to Blair McBride (:Unfocused) from comment #2) > Hm, indeed - the array is changed while it's being iterated over. It really > needs to take a snapshot of the array, and iterate over that. Would You mind to check the iterating of AddonManagerInternal.providers[] too? callProvider() is feeded by providers[]' elements (by forEach loop), but IMO it isn't enough. Thanks!
Assignee | ||
Comment 4•12 years ago
|
||
This ends up making the code read a bit cleaner too, as a nice bonus. Being a bit paranoid in inInstallEnabled etc, but just because you're paranoid doesn't mean they're not out to get you.
Attachment #628201 -
Flags: review?(geoff)
Attachment #628201 -
Flags: review?(dtownsend+bugmail)
Assignee | ||
Comment 5•12 years ago
|
||
Adds a parameter guard to callProviders (bug 746908), and uses the new rest-args JS feature (bug 759446).
Attachment #628201 -
Attachment is obsolete: true
Attachment #628201 -
Flags: review?(geoff)
Attachment #628201 -
Flags: review?(dtownsend+bugmail)
Attachment #628208 -
Flags: review?(geoff)
Attachment #628208 -
Flags: review?(dtownsend+bugmail)
Assignee | ||
Comment 6•12 years ago
|
||
Filed bug 759702 to check XPIProvider for this type of issue.
Updated•12 years ago
|
Attachment #628208 -
Flags: review?(dtownsend+bugmail) → review+
Updated•12 years ago
|
Attachment #628208 -
Flags: review?(geoff) → review+
Assignee | ||
Comment 7•12 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/8cb94b792009
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla15
Comment 8•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/8cb94b792009
Updated•12 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•