Closed Bug 460941 Opened 16 years ago Closed 16 years ago

DIR_Shutdown not always called on shutdown

Categories

(MailNews Core :: Address Book, defect)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED
Thunderbird 3.0b2

People

(Reporter: standard8, Assigned: standard8)

References

Details

Attachments

(2 files, 1 obsolete file)

I've noticed that DIR_Shutdown is not always being called. It happens if you use Thunderbird/SeaMonkey without starting up the address book window, and is a result of our move away from RDF (nsAbDirectoryDataSource is not always being initialised).

This means we leak memory, and we fail to delete the DIRPrefObserver and free the dir_ServerList in nsDirPrefs.

I think this may cause profile switching issues on SeaMonkey (depending on if profile switching is doing a complete shutdown/restart or not).
Flags: wanted-thunderbird3+
Flags: blocking-seamonkey2.0a2?
I tried profile switching a few times with open and non-open mailnews and didn't see problems on the user experience side (didn't/couldn't test leaks). I'm inclined to minus this for the upcoming alpha as I also haven't heard of problems on theis front for others since those changes were made.
Taking off the blocking request list based no comment 1. I have a patch which I'll attach in a while.
Flags: blocking-seamonkey2.0a2? → blocking-seamonkey2.0a2-
Attached patch The fix (obsolete) — Splinter Review
This is what I'm proposing. Since we're moving away from rdf then we need to move management of prefs etc to nsAbManager - so this patch makes that responsible for calling DIR_Shutdown.

nsAbDirectoryDataSource still needs to do its own cleanup for the time being, which is why we don't remove everything from there.

I think this should all be safe, I don't see an issue with moving DIR_Shutdown at that stage.
Assignee: nobody → bugzilla
Status: NEW → ASSIGNED
Attachment #350772 - Flags: superreview?(bienvenu)
Attachment #350772 - Flags: review?(neil)
Attachment #350772 - Flags: superreview?(bienvenu)
Attachment #350772 - Flags: review?(neil)
Attached patch The fix v2Splinter Review
Neil said the first patch was bitrotted. All I've done for this one is regenerate it, not changed anything.
Attachment #350772 - Attachment is obsolete: true
Attachment #351449 - Flags: superreview?(bienvenu)
Attachment #351449 - Flags: review?(neil)
Attachment #351449 - Flags: superreview?(bienvenu) → superreview+
Attachment #351449 - Flags: review?(neil) → review+
This is a supplemental fix for the unit test - before checking in I just realised the import test was failing in debug due to:

###!!! ASSERTION: Using observer service off the main thread!: 'Error', file /Users/moztest/comm/main/src/mozilla/xpcom/ds/nsObserverService.cpp, line 130

This appears to be because we are getting the nsIAbManager within a thread (even though we then proxy to it).

I think we should be safe just to fix the test so that it initialises nsIAbManager before it uses it.
Attachment #351541 - Flags: review?(neil)
(In reply to comment #5)
> This is a supplemental fix for the unit test - before checking in I just
> realised the import test was failing in debug due to:
> 
> ###!!! ASSERTION: Using observer service off the main thread!: 'Error', file
> /Users/moztest/comm/main/src/mozilla/xpcom/ds/nsObserverService.cpp, line 130
Then we should use NS_ENSURE_TRUE(NS_IsMainThread(), NS_ERROR_FAILURE);
Attachment #351541 - Flags: review?(neil) → review+
Comment on attachment 351541 [details] [diff] [review]
Fix Import Unit Test

r=me with nsAbManager::Init fixed to fail off-thread.
Checked in: http://hg.mozilla.org/comm-central/rev/38e8cf481a5b
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.0b2
V.Fixed, per bug 458486 comment 5...
Status: RESOLVED → VERIFIED
Depends on: 509975
You need to log in before you can comment on or make changes to this bug.