Closed Bug 1097528 Opened 10 years ago Closed 4 months ago

An observer notification can cause other observers to be removed/added to the observers array

Categories

(Toolkit :: Places, defect, P5)

defect

Tracking

()

RESOLVED INCOMPLETE

People

(Reporter: mak, Unassigned)

Details

I'm observing this with nsNavHistoryResults in the Library

a removeFolderChildren is ongoing

onItemRemoved for a folder is notified to the left pane, since it has excludeItems and friends, it causes a refresh. The parent of the removed folder is selected.

this causes the right pane to be replaced (even if the location is the same), this version of the right pane is already up-to-date. Though after filling up it registers for notifications.
The previous right pane result unregistered instead.

basically the observers array changes from
[parent, old]
to
[parent new]
and we are notifying parent at the moment

At this point onItemRemoved gets notified to the new result, but it is already up-to-date, so we hit this assertion:
NS_NOTREACHED("Removing item we don't have");

I think we may want to copy the observers array before we start looping through them, though at that point observers could still be removed and then we'd not want to notify those. We could copy the array, and check each entry still exist in the original before notifying, but it's sort of crazy and something we can't do in GetObservers() users. On the other side just copying means that something that unregistered could still get a notification after it unregistered, and that's not nice too, but something we already hit with GetObservers().

For now we could just optimize the right pane to not make a new result if the location is the same.
Priority: -- → P4
Priority: P4 → P5
Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → INACTIVE
Status: RESOLVED → REOPENED
Resolution: INACTIVE → ---
Severity: normal → S3

Unclear if this applies to new notifications, resolving for now.

Status: REOPENED → RESOLVED
Closed: 6 years ago4 months ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.