nsSystemPrincipal confuses the refcount logging

VERIFIED FIXED in mozilla0.9

Status

()

Core
Security: CAPS
P3
normal
VERIFIED FIXED
18 years ago
18 years ago

People

(Reporter: dbaron, Assigned: dbaron)

Tracking

Trunk
mozilla0.9
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

nsSystemPrincipal confuses the XPCOM refcount logging tools, which end up
indicating a refcount of -1, adding noise to the leak stats.  This is because
nsDestroyJSPrincipals adds to the refcount manually and releases using Release()
to delete the object.

The reason for this whole issue is that a single refcount is used for both JS
and XPCOM.  If the refcount hits zero through a JS release, then
nsDestroyJSPrincipals is called, whereas if it hits zero through an XPCOM
release, then the object is destroyed by its Release method.  If the last
release comes through XPCOM, then things are fine, but if the last release comes
through JS, then the XPCOM logging is confused because nsDestroyJSPrincipals
does one more release than addref.  Changing the refcount++ in
nsDestroyJSPrincipals to a real AddRef didn't seem to work when I tried it
before -- I need to investigate more why not, since I think this is the real fix.

I have a hack that works when both the first addref and the last release come
from JS, which seems to be the case currently.  However, I don't like it.
Created attachment 18713 [details] [diff] [review]
fix that I don't like (see description)
Aha.  The reason the AddRef didn't work was because the leak logging considers
AddRef-to-1 special, just like it considers Release-to-0 special.  So there's a
sneaky fix that's much cleaner, about to be attached...
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9
Looks good to me, r=mstoltz.
Sleazy, but WIB.  sr=brendan@mozilla.org.

/be
Fix checked in 2000-11-07 19:06 PDT.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 7

18 years ago
Verified per dbaron@fas.harvard.edu's comments.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.