There are two problems here: 1. You hold an atom in a static nsCOMPtr. You should never have C++ objects with ctors and dtors declared as statics. 2. If you're gonna assign to an nsCOMPtr from NS_NewAtom(), use dont_AddRef()! You should prolly fix this by using a bare pointer to the atom. I'll let you decide...
oh man, I forgot that nsCOMPtrs would have constructors/destructors. I was just being lazy - I'll just use the bare pointer.
Status: NEW → ASSIGNED
Target Milestone: --- → M18
mail triage marking [nsbeta3-]
a leak is a leak is a leak. asking for reconsideration. shouldn't be hard.
*** Bug 50399 has been marked as a duplicate of this bug. ***
ok ok I fixed this :) I have it in my tree but I have fixes to bug 40343 in this file. I'm about to attach the patch to that bug. Patrick/Waterson - care to review the kDefaultServerAtom part of that patch?
Whiteboard: Fix in Hand
Why not just say kDefaultAtom = dont_AddRef(NS_NewAtom("DefaultServer")); rather than converting away from an nsCOMPtr? Seems like a step backward to me.
Yes, it might seem that way... but in fact it was a /static/ nsCOMPtr, which is bad - static objects in general are bad because some platforms have broken library loaders which won't fire the constructors when the library is loaded. (so for example a static nsCOMPtr would not be initialized to null, and it might try to NS_RELEASE() some bad pointer) see chris's comments above in this bug
Hmm, even if the static constructor doesn't fire, it will still be cleared memory. All unitialized static data is at least guaranteed to be zeroed, on every platform I've programmed (except may be 68K code resources, but that doesn't count).
Don't hold an atom in a comptr. It will cause a bunch of assertions to fire ("atom released after atom table destroyed!") and generally makes for noise on the bloat & leak reports.
Yup - also what chris said :) fix is in
Status: ASSIGNED → RESOLVED
Last Resolved: 19 years ago
Resolution: --- → FIXED
Looked at the diff & code, seems good to me. Verifying.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.