Closed
Bug 1003297
Opened 10 years ago
Closed 10 years ago
small cleanups to XPCOM services utilities
Categories
(Core :: XPCOM, defect)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
mozilla32
People
(Reporter: froydnj, Unassigned)
Details
Attachments
(2 files)
2.19 KB,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
1.72 KB,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Reporter | ||
Comment 1•10 years ago
|
||
On a whim, I instrumented nsGetServiceByContractID to print out contract IDs as it was called. I noticed an abnormally high number of observer service contract IDs, which I found odd, given that there's already a cached getter for the observer service in mozilla::services. The reason for this appears to be a lot of things trying to access the observer service *after* mozilla::services::Shutdown. Since the cached pointer is null, we'll try calling into do_GetService and the associated machinery. Of course, since XPCOM is shutdown, we'll just return an error. This patch makes us return an error a little sooner, which might improve shutdown performance slightly (though shutdown is probably dominated by I/O anyway...).
Attachment #8414614 -
Flags: review?(benjamin)
Reporter | ||
Comment 2•10 years ago
|
||
This is really two things: - using swap() instead of forget().take(), which IMHO is a little cleaner; - using nsCOMPtr instead of nsRefPtr, since the TYPE in this instance is an interface type, which we're supposed to be using with nsCOMPtr anyway.
Attachment #8414615 -
Flags: review?(benjamin)
Updated•10 years ago
|
Attachment #8414614 -
Flags: review?(benjamin) → review+
Comment 3•10 years ago
|
||
Comment on attachment 8414615 [details] [diff] [review] part 2 - tidy up refcount management in mozilla::services::Get* I wonder if we should add a NS_WARNING to the other patch: code really shouldn't be asking for services this late in shutdown anyway. Or maybe spend a few minutes in a debugger seeing if there's some common/stupid pattern to it.
Attachment #8414615 -
Flags: review?(benjamin) → review+
Reporter | ||
Comment 4•10 years ago
|
||
(In reply to Benjamin Smedberg [:bsmedberg] from comment #3) > I wonder if we should add a NS_WARNING to the other patch: code really > shouldn't be asking for services this late in shutdown anyway. Or maybe > spend a few minutes in a debugger seeing if there's some common/stupid > pattern to it. I didn't step through several dozen accesses of the observer service in shutdown, but the major things I noticed were: - the cycle collector notifies observers when it's doing things; - pref branches (possibly just the root ones?) remove observers they've added; - nsExpirationTrackers from nsIDocument observers trying to remove themselves; - etc. I think with the current state of things, you'd get a flood of NS_WARNINGs. Maybe some of the removals should be listening for XPCOM shutdown and auto-remove themselves then.
Comment 5•10 years ago
|
||
> the cycle collector notifies observers when it's doing things;
I don't know if it is worth fixing necessarily, but the CC does know when it is in shutdown, so it could just skip observer calls, rather than having them fail.
Comment 6•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/1f6e71553333 https://hg.mozilla.org/mozilla-central/rev/060bea047502
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in
before you can comment on or make changes to this bug.
Description
•