DIR_Shutdown not always called on shutdown

VERIFIED FIXED in Thunderbird 3.0b2

Status

MailNews Core
Address Book
VERIFIED FIXED
10 years ago
9 years ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

Trunk
Thunderbird 3.0b2
Dependency tree / graph
Bug Flags:
wanted-thunderbird3 +
in-testsuite +
blocking-seamonkey2.0a2 -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

10 years ago
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?

Comment 1

10 years ago
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.
(Assignee)

Comment 2

10 years ago
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-
(Assignee)

Comment 3

10 years ago
Created attachment 350772 [details] [diff] [review]
The fix

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)
(Assignee)

Updated

10 years ago
Attachment #350772 - Flags: superreview?(bienvenu)
Attachment #350772 - Flags: review?(neil)
(Assignee)

Comment 4

10 years ago
Created attachment 351449 [details] [diff] [review]
The fix v2

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)

Updated

10 years ago
Attachment #351449 - Flags: superreview?(bienvenu) → superreview+

Updated

10 years ago
Attachment #351449 - Flags: review?(neil) → review+
(Assignee)

Comment 5

10 years ago
Created attachment 351541 [details] [diff] [review]
Fix Import Unit Test

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)

Comment 6

10 years ago
(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);

Updated

10 years ago
Attachment #351541 - Flags: review?(neil) → review+

Comment 7

10 years ago
Comment on attachment 351541 [details] [diff] [review]
Fix Import Unit Test

r=me with nsAbManager::Init fixed to fail off-thread.
(Assignee)

Comment 8

10 years ago
Checked in: http://hg.mozilla.org/comm-central/rev/38e8cf481a5b
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.0b2
Blocks: 458486
V.Fixed, per bug 458486 comment 5...
Status: RESOLVED → VERIFIED

Updated

9 years ago
Depends on: 509975
You need to log in before you can comment on or make changes to this bug.