Firefox 16 beta crash in nsIContent::SetAttr with greasemonkey installed

[njdove]

When Global Menu Bar Integration 3.4.1 is enabled, Firefox 16b3 from the firefox-next PPA crashes shortly after opening. There is a 1:1 correlation between the crashes and this add-on - the open tabs and other add-ons don't have any impact. I'm experiencing this under 64-bit Ubuntu 12.04.

[Chris Coulson (chrisccoulson)]

Thanks, but there's nothing we can do without a stacktrace. If you're experiencing a crash, then please use the crash reporter tool to report it

[Chris Coulson (chrisccoulson)]

And if you're going to report a bug, it's better for you to just report a bug against the "firefox" package in Ubuntu by following the bug reporting guidelines (https://wiki.ubuntu.com/MozillaTeam/Bugs), as this also attaches information about other addons you have installed. Most crashes we see in this addon are triggered by something strange another addon does with its menus, and so a developer normally isn't able to reproduce the issue and isn't able to help the user when such little information is provided (and I'm afraid that we don't have the ability to mind read, although I wish I did sometimes)

[Chris Coulson (chrisccoulson)]

Is this your report?

https://crash-stats.mozilla.com/report/index/0018e20b-d434-4b50-b87e-ff4a42120915

There are 6 reports dated September 15th, which all look like they've been submitted by the same person (although we can't tell for sure, but the combination of addons is identical). But there are also 2 reports from September 11th, with beta 2 as well...

[njdove (njdove)]

Yes, https://crash-stats.mozilla.com/report/index/0018e20b-d434-4b50-b87e-ff4a42120915 is me, as are the other 5 recent reports. The September 11th reports are not mine.

Unfortunately, I am now unable to reproduce the once-reliable crashes. Now that I know how to submit a stacktrace I will do so if the problem recurs. Thank you for your prompt and informative feedback.

[Sami Jaktholm (sjakthol)]

I'm seeing similar crashes (SIGSEGV in nsIContent::SetAttr) with firefox and firefox-globalmenu 15.0.1+build1-0ubuntu0.12.04.1 (current Precise versions). When the crashing starts it happens pretty much every time I open Firefox with the Global Menu extension enabled.

Here's what gdb says (which seem to state problem rather clearly):
Program received signal SIGSEGV, Segmentation fault.
nsIContent::SetAttr (this=0x0, aNameSpaceID=0, aName=0x7fffdcab70c0,
    aValue=..., aNotify=false) at ../../../dist/include/nsIContent.h:347
347 ../../../dist/include/nsIContent.h: No such file or directory.

Full backtrace attached.

[Chris Coulson (chrisccoulson)]

The stacktrace shows clearly what happens. A menu is removing its menupopup in a popuphiding handler, which then causes a null ptr deref when we go to fire the popuphidden event. This should be a trivial fix.

If you scrub through all of the menus with the mouse, does a particular submenu cause the crash when it closes? I'd be interested to know which menu actually triggers this. I don't think anything in Firefox actually does it.

[Chris Coulson (chrisccoulson)]

And thanks for reporting by the way. It's important that we have beta testers to find issues like this, but please do use Apport in future :)

(it even attaches crash ID's automatically, to allow us to easily identify crash reports on https://crash-stats.mozilla.com/ without having to manually search or ask you for more info)

[Sami Jaktholm (sjakthol)]

It's funny how the crashing stops when I really need it to crash. Well, at least it's safe to say that it's caused by something that's not present in default menu structure. But when the crashing starts again, I'll try to find out what's causing the crashing...

[Chris Coulson (chrisccoulson)]

Heh, no worries, thanks!

I'll fix this for the next beta anyway

[Sami Jaktholm (sjakthol)]

Alright, it didn't take that long the get it crashing again. After looking into the traffic of the DBusmenu of Firefox, it seems obvious that hud-service is causing these crashes.

When Firefox is launched HUD starts to map the menus. HUD iterates trough the menu structure and sends 'opened' events to all the sub-menus in need of mapping.

When the focus is changed to another window, HUD wants to close the opened menus. So it sends 'closed' event to all the menus and Firefox responds to that request. But here's the problem: it's closing more menus it opened. For me, it sent 'opened' event to 40 objects but sends 'closed' event to 46 objects. And Firefox crashes.

Disabling add-ons one by one I concluded that it's Greasemonkey causing the crashes (which is also in the list of extensions of the original reporter). Greasemonkey adds its own sub-menu to Tools which happen to have exactly 6 menu items: the same number of objects as the extra ones HUD tries to close. So my guess is that HUD considers those items normal menus and tries to close them (why this happens, I have no idea).

Here's steps to reproduce the crash:
#1: Have Greasemonkey installed and enabled
#2: Open Firefox and wait for the window to appear
#3: Invoke HUD with ALT and perform a search (might not be needed if hud-service was already running)
#4: Give focus to another application
#5: Firefox crashes

The reason the crash didn't happen all the time was that I hadn't used HUD after booting my computer. So the hud-service wouldn't try to close any menus and the crash would not happen.

[Chris Coulson (chrisccoulson)]

Thanks for that.

The HUD iterating over the menus at startup seems like another bug. That's quite undesirable as it puts menu building on the critical path at startup, which we really want to avoid (with the startup time budget being so small already).

Cc'ing Ryan for that though. I'm not sure if that's a bug in the HUD

[Sami Jaktholm (sjakthol)]

Now that you mentioned it, the HUD loading menus at startup is quite a weird one too. It only happens if you had searched something with HUD and activated a result or performed a search and closed HUD by clicking outside HUD area.

If you use Escape to close the HUD, the menus will only be loaded once HUD actually needs them. It seems that somehow HUD thinks it needs to know what the menus have ASAP (it's probably left on some kind of a search mode)

But I'll open a bug against Unity for that one...

[Chris Coulson (chrisccoulson)]

We're getting this on 15.0.1 as well now....

[Chris Coulson (chrisccoulson)]

This has basically exploded for 15.0.1 now

[Launchpad Janitor (janitor)]

This bug was fixed in the package firefox - 16.0+build1-0ubuntu1

---------------
firefox (16.0+build1-0ubuntu1) quantal-security; urgency=low

  * New upstream stable release (FIREFOX_16_0_BUILD1)
    - see LP: #1062611 for USN information

  * Update globalmenu-extension to 3.5.4
    - Fix crash in uGlobalMenuDocListener::HandleMutations
  * Drop StartupWMClass from the desktop file, as it isn't actually
    required by gnome shell or unity
    - update debian/firefox.desktop.in
  * Refresh patches
    - update debian/patches/ubuntu-ua-string-changes.patch
    - update debian/patches/dont-override-general-useragent-locale.patch
  * Drop compare-locales from the packaging, and just check out the current
    version when we create the tarball
    - update debian/build/mozbuild.mk
    - update debian/build/create-tarball.py
    - remove debian/build/compare-locales
  * Generate a list of the search engines that we modify and store this
    in bzr, rather than determining this list at build time. We refresh
    the list in the clean target and fail the build if anything changes.
    This ensures that any upstream changes to the search engine list
    for any locale will not go unnoticed and will force a manual check
    to ensure that we are shipping correctly modified search engines
    - add debian/build/refresh-search-modifications.pl
    - update debian/build/mozbuild.mk
    - add debian/config/search-mods.list
    - rename debian/build/check-search-overrides.pl =>
        debian/build/verify-search-overrides.pl
  * Drop obsolete autocomplete-theme-tweak patch now
    - remove debian/patches/autocomplete-theme-tweak.patch
    - update debian/patches/series
  * Fix for bmo: #795395
    - add debian/patches/fix-for-bmo795395.patch
    - update debian/patches/series
 -- Chris Coulson <email address hidden> Fri, 05 Oct 2012 19:16:08 +0100

[Launchpad Janitor (janitor)]

This bug was fixed in the package firefox - 16.0+build1-0ubuntu0.11.04.1

---------------
firefox (16.0+build1-0ubuntu0.11.04.1) natty-security; urgency=low

  * New upstream stable release (FIREFOX_16_0_BUILD1)
    - see LP: #1062611 for USN information

  * Update globalmenu-extension to 3.5.4
    - Fix LP: #1051152 - Crash in nsIContent::SetAttr with greasemonkey
      installed
    - Improve handling of radio items, and work correctly for radio items
      without a name (fixes the View -> Page Style submenu)
    - Stop causing nested DOM mutations. There is a big warning about doing
      this in nsIMutationObserver.h
    - Delay processing of DOM mutations during code sections when we are
      dispatching events. This should prevent a recurrence of
      reentrancy-triggered crashes such as LP: #1025011, LP: #1035305 and
      LP: #1051152, which have been exposed by updates of third-party addons
      recently and caused because our view of the menu changes during
      event delivery
    - Fix crash in uGlobalMenuDocListener::HandleMutations
  * Fix for bmo: #795395
    - add debian/patches/fix-for-bmo795395.patch
    - update debian/patches/series
  * Refresh patches after merging of nsILocalFile in to nsIFile
    - update debian/patches/firefox-kde.patch
    - update debian/patches/mozilla-kde.patch
  * Drop StartupWMClass from the desktop file, as it isn't actually
    required by gnome shell or unity
    - update debian/firefox.desktop.in
  * Refresh patches
    - update debian/patches/ubuntu-ua-string-changes.patch
    - update debian/patches/mozilla-kde.patch
    - update debian/patches/dont-override-general-useragent-locale.patch
    - update debian/patches/firefox-kde.patch
  * Drop compare-locales from the packaging, and just check out the current
    version when we create the tarball
    - update debian/build/mozbuild.mk
    - update debian/build/create-tarball.py
    - remove debian/build/compare-locales
  * Generate a list of the search engines that we modify and store this
    in bzr, rather than determining this list at build time. We refresh
    the list in the clean target and fail the build if anything changes.
    This ensures that any upstream changes to the search engine list
    for any locale will not go unnoticed and will force a manual check
    to ensure that we are shipping correctly modified search engines
    - add debian/build/refresh-search-modifications.pl
    - update debian/build/mozbuild.mk
    - add debian/config/search-mods.list
    - rename debian/build/check-search-overrides.pl =>
        debian/build/verify-search-overrides.pl
  * Drop obsolete autocomplete-theme-tweak patch now
    - remove debian/patches/autocomplete-theme-tweak.patch
    - update debian/patches/series
 -- Chris Coulson <email address hidden> Tue, 28 Aug 2012 11:56:44 +0100

[Launchpad Janitor (janitor)]

This bug was fixed in the package firefox - 16.0+build1-0ubuntu0.12.04.1

---------------
firefox (16.0+build1-0ubuntu0.12.04.1) precise-security; urgency=low

  * New upstream stable release (FIREFOX_16_0_BUILD1)
    - see LP: #1062611 for USN information

  * Update globalmenu-extension to 3.5.4
    - Fix LP: #1051152 - Crash in nsIContent::SetAttr with greasemonkey
      installed
    - Improve handling of radio items, and work correctly for radio items
      without a name (fixes the View -> Page Style submenu)
    - Stop causing nested DOM mutations. There is a big warning about doing
      this in nsIMutationObserver.h
    - Delay processing of DOM mutations during code sections when we are
      dispatching events. This should prevent a recurrence of
      reentrancy-triggered crashes such as LP: #1025011, LP: #1035305 and
      LP: #1051152, which have been exposed by updates of third-party addons
      recently and caused because our view of the menu changes during
      event delivery
    - Fix crash in uGlobalMenuDocListener::HandleMutations
  * Fix for bmo: #795395
    - add debian/patches/fix-for-bmo795395.patch
    - update debian/patches/series
  * Drop StartupWMClass from the desktop file, as it isn't actually
    required by gnome shell or unity
    - update debian/firefox.desktop.in
  * Refresh patches
    - update debian/patches/ubuntu-ua-string-changes.patch
    - update debian/patches/dont-override-general-useragent-locale.patch
  * Drop compare-locales from the packaging, and just check out the current
    version when we create the tarball
    - update debian/build/mozbuild.mk
    - update debian/build/create-tarball.py
    - remove debian/build/compare-locales
  * Generate a list of the search engines that we modify and store this
    in bzr, rather than determining this list at build time. We refresh
    the list in the clean target and fail the build if anything changes.
    This ensures that any upstream changes to the search engine list
    for any locale will not go unnoticed and will force a manual check
    to ensure that we are shipping correctly modified search engines
    - add debian/build/refresh-search-modifications.pl
    - update debian/build/mozbuild.mk
    - add debian/config/search-mods.list
    - rename debian/build/check-search-overrides.pl =>
        debian/build/verify-search-overrides.pl
  * Drop obsolete autocomplete-theme-tweak patch now
    - remove debian/patches/autocomplete-theme-tweak.patch
    - update debian/patches/series
 -- Chris Coulson <email address hidden> Fri, 05 Oct 2012 19:35:49 +0100

[Launchpad Janitor (janitor)]

This bug was fixed in the package firefox - 16.0+build1-0ubuntu0.11.10.1

---------------
firefox (16.0+build1-0ubuntu0.11.10.1) oneiric-security; urgency=low

  * New upstream stable release (FIREFOX_16_0_BUILD1)
    - see LP: #1062611 for USN information

  * Update globalmenu-extension to 3.5.4
    - Fix LP: #1051152 - Crash in nsIContent::SetAttr with greasemonkey
      installed
    - Improve handling of radio items, and work correctly for radio items
      without a name (fixes the View -> Page Style submenu)
    - Stop causing nested DOM mutations. There is a big warning about doing
      this in nsIMutationObserver.h
    - Delay processing of DOM mutations during code sections when we are
      dispatching events. This should prevent a recurrence of
      reentrancy-triggered crashes such as LP: #1025011, LP: #1035305 and
      LP: #1051152, which have been exposed by updates of third-party addons
      recently and caused because our view of the menu changes during
      event delivery
    - Fix crash in uGlobalMenuDocListener::HandleMutations
  * Fix for bmo: #795395
    - add debian/patches/fix-for-bmo795395.patch
    - update debian/patches/series
  * Refresh patches after merging of nsILocalFile in to nsIFile
    - update debian/patches/firefox-kde.patch
    - update debian/patches/mozilla-kde.patch
  * Drop StartupWMClass from the desktop file, as it isn't actually
    required by gnome shell or unity
    - update debian/firefox.desktop.in
  * Refresh patches
    - update debian/patches/ubuntu-ua-string-changes.patch
    - update debian/patches/mozilla-kde.patch
    - update debian/patches/dont-override-general-useragent-locale.patch
    - update debian/patches/firefox-kde.patch
  * Drop compare-locales from the packaging, and just check out the current
    version when we create the tarball
    - update debian/build/mozbuild.mk
    - update debian/build/create-tarball.py
    - remove debian/build/compare-locales
  * Generate a list of the search engines that we modify and store this
    in bzr, rather than determining this list at build time. We refresh
    the list in the clean target and fail the build if anything changes.
    This ensures that any upstream changes to the search engine list
    for any locale will not go unnoticed and will force a manual check
    to ensure that we are shipping correctly modified search engines
    - add debian/build/refresh-search-modifications.pl
    - update debian/build/mozbuild.mk
    - add debian/config/search-mods.list
    - rename debian/build/check-search-overrides.pl =>
        debian/build/verify-search-overrides.pl
  * Drop obsolete autocomplete-theme-tweak patch now
    - remove debian/patches/autocomplete-theme-tweak.patch
    - update debian/patches/series
 -- Chris Coulson <email address hidden> Tue, 28 Aug 2012 11:56:44 +0100