"ASSERTION: Event listener manager hash not empty at shutdown!" with Flash and Greasemonkey

RESOLVED FIXED in mozilla1.9beta2



12 years ago
11 years ago


(Reporter: jruderman, Assigned: jruderman)


(Blocks: 1 bug, {assertion, memory-leak, testcase})

assertion, memory-leak, testcase
Dependency tree / graph
Bug Flags:
blocking1.9 +
in-testsuite ?

Firefox Tracking Flags

(Not tracked)



(9 attachments)



12 years ago
Loading the testcase (in Mac trunk debug Firefox) triggers:

###!!! ASSERTION: Event listener manager hash not empty at shutdown!: 'sEventListenerManagersHash.entryCount == 0', file /Users/jruderman/trunk/mozilla/content/base/src/nsContentUtils.cpp, line 708

It also makes Firefox leak 1 nsEventListenerManager, 1 nsGenericElement, 1 nsHTMLSharedObjectElement, and a bunch of other objects.

Comment 1

12 years ago
Posted file testcase

Comment 2

12 years ago
You'll get that assert any time you leak an element, basically.

Can you get a balance tree for that nsHTMLSharedObjectElement leak?  And possibly for the nsPluginInstanceOwner and ns4xPluginInstance?

I would assume the  nsPluginInstanceOwner leaks the element, but not sure how the plugin stuff ends up leaking itself here.

Comment 4

12 years ago
This doesn't leak for me.

Comment 5

12 years ago
This is my first time trying to make a balance tree, so I don't know whether I did it correctly.
Hmm.  What command line did you use?  That tree has some really odd things going on...

Comment 7

12 years ago
The command line is included in the attachment.
Ah, ok. Try something more like:

  export XPCOM_MEM_REFCNT_LOG=log-file.dat
  export XPCOM_MEM_COMPTR_LOG=log-comptr.dat
  export XPCOM_MEM_LOG_CLASSES=nsHTMLSharedObjectElement

Run firefox.  Run find-leakers.pl on log-file.dat to find the address.

  make-tree.pl --object ADDRESS_HERE --comptr log-comptr.dat --ignore-balanced < ~/log-file.dat | fix-macosx-stack.pl

The difference with what you did is logging all the nsCOMPtr activity and getting rid of the matching addrefs/releases from that even if they're in different parts of the balance tree.  Makes the tree smaller and simpler to read.

Comment 9

12 years ago
Ignoring nsCOMPtr activity made the whole tree go away?

Comment 10

12 years ago
This log makes more sense.  There's even some stuff at the bottom involving "scroll position listeners".
(In reply to comment #9)
> Ignoring nsCOMPtr activity made the whole tree go away?

When that's the case, generally find-comptr-leakers will show some leaks.

Comment 12

12 years ago
find-comptr-leakers.pl tells me that it's the nsPluginInstanceOwner's mContent pointer that continues to hold onto the nsHTMLSharedObjectElement.

Comment 13

12 years ago
nsPluginInstanceOwner::ScrollPositionDidChange's use of timers is Mac-specific.
Note that the leak log (XPCOM_MEM_LEAK_LOG) will say for the leaked things how many refs were from COMPtrs, so you can tell whether to use find-comptr-leakers or find-leakers.
Hmm.  So there are all sorts of CancelTimer() calls in nsPluginInstanceOwner.  So I'd expect this cycle to be broken...  Why aren't we hitting any of them?  Should the frame Destroy() method make sure the timer gets killed?
Flags: blocking1.9?
And for what it's worth, I can't reproduce on Mac either.

Comment 17

12 years ago
Hmm, so it's up to me to debug it, then?
If it's consistently reproducible, yes....

I'd start by adding printfs to the places where the timer is set up and to CancelTimer and seeing whether the latter is being hit at all, I guess.

Does this perhaps depend on Flash version?
Jesse, here's your first blocker :)

Let me know if you need help and I'll do my best.
Assignee: nobody → jruderman
Flags: blocking1.9? → blocking1.9+

Comment 20

12 years ago
I can't reproduce in a completely clean profile, but once I install Greasemonkey (even without installing any user scripts), I can reproduce.

It seems like two nsPluginInstanceOwner objects are being created for the same frame, and one of them isn't getting torn down properly.  More soon.
Summary: "ASSERTION: Event listener manager hash not empty at shutdown!" with Flash → "ASSERTION: Event listener manager hash not empty at shutdown!" with Flash and Greasemonkey

Comment 21

12 years ago
I might have figured out what's going wrong.  nsPluginInstanceOwner::PrepareToStop is called on the first nsPluginInstanceOwner object, but mWidget is null, so PrepareToStop bails out early and doesn't unregister its scroll listener.  Later, the scroll listener is notified, setting up a cycle between the timer and the nsPluginInstanceOwner.  nsPluginInstanceOwner::Destroy doesn't get to break this cycle because it is called before the cycle is set up.

If I change the mWidget null check in nsPluginInstanceOwner::PrepareToStop to only wrap the code added in bug 347743 (which uses mWidget) instead of bailing early (and skipping the "Unregister scroll position listener" code), the leak goes away.  Eli and jst, do you remember why you made the null check do that in bug 347743?

I don't know why Greasemonkey causes two nsPluginInstanceOwners to be created, though.  I wonder if that's related to bug 390183.
Blocks: 347743

Comment 22

12 years ago
This is what I used to debug the leak so far.

Comment 23

12 years ago

Comment 24

12 years ago
Attachment #287474 - Flags: superreview?(jst)
Attachment #287474 - Flags: review?(sharparrow1)
Attachment #287474 - Flags: superreview?(jst) → superreview+


12 years ago
Target Milestone: --- → mozilla1.9 M10


12 years ago
> It seems like two nsPluginInstanceOwner objects are being created 

What are the stacks to those creations?
Uh.  So we reenter nsObjectFrame::Instantiate, looks like.  I think we need to flag ourselves as being inside Instantiate and bail out if it's reentered.  Probably a separate (blocking1.9) bug on that?

It looks like greasemonkey installs a content policy implementation?  I wonder what they do in it...
Huh.  OK, that's kinda cute.

Comment 30

12 years ago
Are Greasemonkey's nsIContentPolicy callbacks doing anything that's disallowed?   Do we have assertions to catch that kind of thing?

Boris, can you file the bug you mentioned in comment 27?  I think you can describe it better than I can.
> Are Greasemonkey's nsIContentPolicy callbacks doing anything that's disallowed?

Not offhand, no.

> Do we have assertions to catch that kind of thing?

For the most part, yeah.

I filed bug 402937.

Attachment #287474 - Flags: review?(sharparrow1) → review+

Comment 32

12 years ago
Patch checked in.

For automated testing: once bug 397725 is fixed, I can check the testcase in as a crashtest, but the tests would have to be run with Greasemonkey installed to trigger the assertion and leak.  Hmm.
Group: security
Last Resolved: 12 years ago
Flags: in-testsuite?
Resolution: --- → FIXED


11 years ago
Group: security
You need to log in before you can comment on or make changes to this bug.