always leak bindingattached/detached nsXBLEventHandlers

VERIFIED FIXED in Future

Status

()

Core
XBL
P3
normal
VERIFIED FIXED
18 years ago
17 years ago

People

(Reporter: dbaron, Assigned: David Hyatt)

Tracking

({mlk})

Trunk
Future
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [rtm+ need info])

Attachments

(1 attachment)

(Reporter)

Description

18 years ago
Starting sometime between 2000-08-02 23:26 and 2000-08-03 00:18, we leak an
nsXBLEventHandler every time the browser is run.

The checkins during that perioud were mostly ben's UI changes:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2000-08-02+23%3A16&maxdate=2000-08-03+00%3A28&cvsroot=%2Fcvsroot

The leaked object is the first one created (well, really the first one whose
|AddRef| method is called).  It is |AddRef|d once and never |Release|d.  The
|AddRef| is here:

<nsXBLEventHandler> 0x08688DE8 1 AddRef 1
nsXBLEventHandler::AddRef(void)+0x0000006F
NS_NewXBLEventHandler(nsIContent *, nsIContent *, nsString const &,
nsXBLEventHandler **)+0x00000064
nsXBLBinding::InstallEventHandlers(nsIContent *, nsIXBLBinding **)+0x0000069D
nsXBLService::LoadBindings(nsIContent *, nsString const &, int, nsIXBLBinding
**)+0x000009B8
nsCSSFrameConstructor::ConstructFrameInternal(nsIPresShell *, nsIPresContext *,
nsFrameConstructorState &, nsIContent *, nsIFrame *, nsIAtom *, int,
nsIStyleContext *, nsFrameItems &, int)+0x0000014F
nsCSSFrameConstructor::ConstructFrame(nsIPresShell *, nsIPresContext *,
nsFrameConstructorState &, nsIContent *, nsIFrame *, nsFrameItems &)+0x00000202
nsCSSFrameConstructor::ProcessChildren(nsIPresShell *, nsIPresContext *,
nsFrameConstructorState &, nsIContent *, nsIFrame *, int, nsFrameItems &, int,
nsTableCreator *)+0x000001F7
nsCSSFrameConstructor::ConstructDocElementFrame(nsIPresShell *, nsIPresContext
*, nsFrameConstructorState &, nsIContent *, nsIFrame *, nsIStyleContext *,
nsIFrame *&)+0x00000859
nsCSSFrameConstructor::ContentInserted(nsIPresContext *, nsIContent *,
nsIContent *, int, nsILayoutHistoryState *)+0x00000A56
StyleSetImpl::ContentInserted(nsIPresContext *, nsIContent *, nsIContent *,
int)+0x00000037
PresShell::InitialReflow(int, int)+0x00000416
nsXULDocument::StartLayout(void)+0x000006D6
nsXULDocument::ResumeWalk(void)+0x000015D9
nsXULDocument::EndLoad(void)+0x0000065D
XULContentSinkImpl::DidBuildModel(int)+0x0000008E
CWellFormedDTD::DidBuildModel(unsigned int, int, nsIParser *, nsIContentSink
*)+0x0000005B
nsParser::DidBuildModel(unsigned int)+0x00000076
nsParser::ResumeParse(int, int)+0x00000236
nsParser::OnStopRequest(nsIChannel *, nsISupports *, unsigned int, unsigned
short const *)+0x000000E3
nsResChannel::EndRequest(unsigned int, unsigned short const *)+0x0000005A
nsResChannel::OnStopRequest(nsIChannel *, nsISupports *, unsigned int, unsigned
short const *)+0x00000100
nsFileChannel::OnStopRequest(nsIChannel *, nsISupports *, unsigned int, unsigned
short const *)+0x00000081
nsOnStopRequestEvent::HandleEvent(void)+0x00000125
nsStreamListenerEvent::HandlePLEvent(PLEvent *)+0x0000005F
PL_HandleEvent+0x00000056
PL_ProcessPendingEvents+0x00000110
nsEventQueueImpl::ProcessPendingEvents(void)+0x00000071
nsAppShell::SetDispatchListener(nsDispatchListener *)+0x0000003C
keysym2ucs+0x0000015F
g_io_add_watch+0x000000AA
g_get_current_time+0x00000136
g_get_current_time+0x00000701
g_main_run+0x00000081
gtk_main+0x000000B9
nsAppShell::Run(void)+0x00000052
nsAppShellService::Run(void)+0x0000003C
UNKNOWN 0x08053305
UNKNOWN 0x080539D8
__libc_start_main+0x000000FF
(Reporter)

Updated

18 years ago
Keywords: mlk

Comment 1

18 years ago
One per launch?  How bad is that?
(Reporter)

Comment 2

18 years ago
One per whatever we do in the bloat tests.  I don't know what it is or how often
it happens in normal use.

Comment 3

18 years ago
I see this regularly when running ...

Comment 4

18 years ago
12 bytes or so per window, ->future.  Sorry Bruce, but dbaron will probably fix
this anyway!
Target Milestone: --- → Future
(Reporter)

Comment 5

18 years ago
not this one...
(Assignee)

Updated

17 years ago
Status: NEW → ASSIGNED
Summary: always leak an nsXBLEventHandler → always leak bindingattached/detached nsXBLEventHandlers
(Assignee)

Comment 6

17 years ago
*** Bug 55006 has been marked as a duplicate of this bug. ***

Comment 7

17 years ago
I futured this bug when it was 12 bytes per window. Current TBox leaky tests
seem to show it much worse recently. What is the leak rate now?

Comment 8

17 years ago
asking for rtm, this is a root leak that is causing other leaks (thousands of
bytes).
Keywords: rtm

Comment 9

17 years ago
so if this object is leaked once per window, and we've attatched 8k more bytes
to this object, that yeilds ~8k per window now. It sounds like Hyatt has a fix
in mind, I say we take it to remove the possibility of it becoming 100k per
window down the line.

Comment 10

17 years ago
adding self to cc list - I'm wondering if fixing this will clean up some thread
pane leaks after scrolling, since that does involve event handlers.
(Assignee)

Comment 11

17 years ago
Created attachment 16058 [details] [diff] [review]
Patch to fix leak.

Comment 12

17 years ago
Cool, this does seem to fix the leak of nsXULDocuments when you scroll the
thread pane with the thumb (which caused all the nsMsgHdrs to get leaked, etc).
So, if this was what fixed this leak (and not some earlier checkin), at least
for mail/news, this leak was more like megabytes then k's. Thanks for fixing this!

Comment 13

17 years ago
rtm+ need info, get reviewer and super-approver to add their a=/r=, then remove
'need info'
Whiteboard: [rtm+ need info]
(Assignee)

Comment 14

17 years ago
This was fixed when I landed 53417 on the branch.  I wasn't going to land a 
leaky 53417 patch on the branch, nor was I going to deliberately unwind my XBL 
files back to the leaky state just so that I could land 53417.

So this fix just went in on the branch.  Marking fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 15

17 years ago
dbaron: is this leak fixed (I believe it is).
Keywords: vtrunk
(Reporter)

Comment 16

17 years ago
Not in tinderbox leak stats -> VERIFIED, removing vtrunk kw.
Status: RESOLVED → VERIFIED
Keywords: vtrunk
You need to log in before you can comment on or make changes to this bug.