Closed Bug 341161 Opened 15 years ago Closed 15 years ago
XPInstall Manager doesnt shutdown correctly .
If an nsIXPInstallManager is created and inited through xpcom then it will fail to shutdown correctly and stop observing the xpinstall-progress topic.
When creating an instance of nsIXPInstallManager and calling either of the init methods available through xpcom, mNeedsShutdown is never set to true. This means that when the manager shutsdown it actually skips all of the shutdown in particular it doesn't attempt to stop observing the xpinstall-progress topic and so can start doing strange things when a new extension installation starts. This patch sets mNeedsShutdown to true and also calls shutdown in the places where the initialisation fails.
Since the manager addrefs itself in the constructor and then doesn't release itself again unless properly shutdown I guess this is going to be a memory leak.
This is needed to fix bug 335757
dveditz, can you please review this ASAP? Pretty major user impact here ...
Target Milestone: --- → mozilla1.8.1beta1
Comment on attachment 225192 [details] [diff] [review] Set mNeedsShutdown in init >- return Observe(aListener, XPI_PROGRESS_TOPIC, NS_LITERAL_STRING("open").get()); >+ rv = Observe(aListener, XPI_PROGRESS_TOPIC, NS_LITERAL_STRING("open").get()); >+ if (NS_FAILED(rv)) >+ Shutdown(); >+ return rv; My only worry here is that Observe might return a failure while also calling DownloadNext() -- that might lead to crashes, too. XPInstall is perfectly happy without a dialog to update so calling DownloadNext() even if we can't get a proxy to the dialog is OK -- but now you probably don't want that to return a failure and trigger a shutdown. r/sr=dveditz **IFF** you change nsXPInstallManager::Observe() to not set rv on the rv = pmgr->GetProxyForObject(... dlg ...)
Made the suggested change, carrying over r/sr.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed]
This is exactly the same fix ported to the branch. The differences are caused by the nsIThreadManager landing. Since this does exactly the same as the previous patch I don't believe that there is a need to re-request review.
Comment on attachment 230168 [details] [diff] [review] Branch patch a=darin on behalf of drivers
Attachment #230168 - Flags: approval1.8.1? → approval1.8.1+
Checked in to MOZILLA_1_8_BRANCH
Whiteboard: [checkin needed (1.8 branch)]
You need to log in before you can comment on or make changes to this bug.