Closed Bug 369336 Opened 17 years ago Closed 17 years ago

XPCOM cycle collector is initialized statically

Categories

(Core :: XPCOM, defect)

x86
All
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: Waldo, Assigned: jaas)

References

()

Details

Attachments

(1 file, 1 obsolete file)

For reasons I don't understand, static construction of XPCOM objects is bad.  Maybe it's because it occurs before enough infrastructure is in place, or maybe it's some other reason.  What I do know, however, is that this static construction causes a warning in debug builds -- my console is filled with them (not all due to the cycle collector, of course) at startup:

WARNING: XPCOM objects created/destroyed from static ctor/dtor: 'gActivityTLS != BAD_TLS_INDEX && NS_PTR_TO_INT32(PR_GetThreadPrivate(gActivityTLS)) == 0', file m:/trunk/mozilla/xpcom/base/nsTraceRefcntImpl.cpp, line 1085

bsmedberg added this some time ago, and I seem to recall he wanted to make it an assertion eventually once in-tree instances were fixed.  sCollector probably needs to be a static pointer which is initialized sometime in the startup process.
Depends on: 362318
the problem is that the object can easily outlast the related shutdown bits, resulting in crashes on exit.

(just spent time switching from NS_WARN_IF_FALSE to NS_ASSERTION to find out that this is already filed.)
OS: Linux → All
Attached patch fix v1.0 (obsolete) — Splinter Review
Please comment on setting sCollector to nsnull in the nsCycleCollector dtor. That seems like a good idea but I want to make sure that in particular gets reviewed.
Assignee: nobody → joshmoz
Status: NEW → ASSIGNED
Attachment #258450 - Flags: review?(graydon)
We may want to do something like this in the dtor instead, in case more than one nsCycleCollector object ever exists:

nsCycleCollector::~nsCycleCollector()
{
    if (this == sCollector)
        sCollector = nsnull;
    ...
}
Attached patch fix v1.1Splinter Review
I think the idea in comment #3 is the way to go. This patch does a check in the dtor before nulling out the static nsCycleCollector.
Attachment #258450 - Attachment is obsolete: true
Attachment #258496 - Flags: review?(graydon)
Attachment #258450 - Flags: review?(graydon)
Comment on attachment 258496 [details] [diff] [review]
fix v1.1

Looks fine, assuming it runs.
Attachment #258496 - Flags: review?(graydon) → review+
Attachment #258496 - Flags: superreview?(vladimir)
Comment on attachment 258496 [details] [diff] [review]
fix v1.1

Looks fine, same comment as graydon's :)
Attachment #258496 - Flags: superreview?(vladimir) → superreview+
fixed on trunk
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: