Closed Bug 15824 Opened 26 years ago Closed 26 years ago

bad refcounting in nsCodebasePrincipal

Categories

(Core :: Security, defect, P3)

x86
Windows NT
defect

Tracking

()

VERIFIED WORKSFORME

People

(Reporter: warrensomebody, Assigned: norrisboyd)

References

Details

Bloaty: Refcounting and Memory Bloat Statistics |<-------Name------>|<--------------References-------------->|<---------------- Objects---------------->|<------Size----->| Rem Total Mean StdDev Rem Total Mean StdDev Per-Class Rem 78 nsCodebasePrincipal -3 258 ( -1.33 +/- 1.90) 1 5 ( 0.56 +/- 0.83) 36 36 Norris, I suspect that the refcount is going negative on this class because the mRefCnt field isn't really being used, and the mJSPrincipals.refcount field is instead. Maybe somewhere along the way, the nsCodebasePrincipal is getting QI'd to nsISupports and that's incrementing the other mRefCnt(?). I couldn't catch it in the act. We really should fix this so that it doesn't get in the way of tracking down other refcounting problems. Warren
Do you have any test case that reproduces this?
There's only one vtable pointer in the nsCodebasePrincipal object, and it points to the overridden forms of AddRef and Release. So a QI should never result in an object whose AddRef and Release affect nsISupports::mRefCnt.
Ok, well I guess that theory's wrong. But I got this by recompiling with BLOATY defined in nsISupportsUtil.h and visiting www.mozilla.org, www.yahoo.com and then drilling down into the yahoo site for several pages. I'll try to reproduce it again later today.
Status: NEW → ASSIGNED
I just checked in changes to assert if Release or AddRef of a nsCodebasePrincipal is called with a mRefcnt != 0. We'll see if anyone else can reproduce this problem.
Well, that's not really what I had in mind when filing this bug. I'd like to see you fix the bloat statistics to reflect reality so that people won't think that nsCodebasePrincipals are leaking. The statistics can get off if the NS_IMPL_ADDREF/RELEASE macros aren't used, or if the NS_LOG_ADDREF/NS_LOG_RELEASE macros aren't used consistently in hand-written AddRef/Release implementations. The statistics can also get off if there's a real leak. Also, I'm not sure your assertion is useful for the case where the nsCodebasePrincipal isn't getting released/destroyed.
Depends on: 16466
I tried turning on the about:bloat code to look at the nsCodebasePrincipal refcounts. I tried loading www.mozilla.org, then www.yahoo.com, and then going down several layers deep into yahoo. I never saw any negative refcounts in the about:bloat page. I did see leaks of nsDocuments, which are holding refs to nsCodebasePrincipals and preventing them from being released. I've filed bug 16466 on that. When that bug is resolved, I'll ensure again that nsCodebasePrincipals are indeed released after the associated page goes away.
Ok, I can't reproduce this now either. Maybe something else got fixed and cleared it up. Maybe we should mark it WORKSFORME.
Status: ASSIGNED → RESOLVED
Closed: 26 years ago
Resolution: --- → WORKSFORME
Done. Thanks for retrying.
Blocks: 16654
Verified worksforme.
Status: RESOLVED → VERIFIED
QA Contact: dshea → junruh
Bulk moving all Browser Security bugs to new Security: General component. The previous Security component for Browser will be deleted.
Component: Security → Security: General
You need to log in before you can comment on or make changes to this bug.