341537 - Leaking factories/modules for some components

Status: RESOLVED FIXED

(Calendar :: Internal Components, defect)

Description

[Joey Minta]

See bug 168411 comment #28.  Gavin reports that if the factory is in the same object as the module, we leak.  This is exactly how some of our component registration is done.  (see for example http://landfill.mozilla.org/mxr-test/mozilla/source/calendar/providers/caldav/calDavCalendar.js#904 )

For an example of what we should do, see http://lxr.mozilla.org/mozilla1.8/source/browser/components/search/nsSearchService.js#2290

Comment 1

[Reed Loden [:reed]]

The bugspam monkeys have struck again. They are currently chewing on default assignees for Calendar. Be afraid for your sanity!

Comment 2

[Stefan Sitter [:ssitter]]

Attachment: [checked in] define factory method outside the module, rev0

Comment 3

[Daniel Boelzle [:dbo]]

Comment on attachment 281463 [details] [diff] [review]
[checked in] define factory method outside the module, rev0

looks good, thanks for the patch; r=dbo

Comment 4

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Is there an updated status for this bug? (I'm not sure if the patch was ever checked in...)

Comment 5

[Philipp Kewisch [:Fallen][📱📆]]

The patch seems to be checked in, but we do have a few locations where the factory is at least created from inside the module (i.e our createFactoryFor methods).

It would be nice to find out if this still causes a leak on 1.9.x or if it was fixed in the meanwhile.

Comment 6

[Philipp Kewisch [:Fallen][📱📆]]

While you are at it, you could also make sure the calItemModule's module correctly implements unregisterSelf(). Seems this does no harm at the moment, but we should be future proof.

Comment 7

[Philipp Kewisch [:Fallen][📱📆]]

The rest of this is obsolete by bug 576746, marking FIXED since a patch was checked in for this bug.