345660 - Crash [@ ClassifyWrapper] with xmlhttprequest and NS_ERROR_FILE_NOT_FOUND when Firebug extension is installed

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect, P1)

Description

[Martijn Wargers (dead)]

See upcoming testcase.
This crashes current Firefox builds (1.8.1 branch and trunk) when the Firebug extension is installed and running.

This regressed on trunk between 2006-05-25 and 2006-05-26.
On branch, the regression happened between 2006-06-06 and 2006-06-26.
I think this regressed with bug 206520.

Talkback ID: TB21297965H
ClassifyWrapper   nsDOMClassInfo::BeginGCMark   nsDOMClassInfo::MarkReachablePreservedWrappers   nsDOMGCParticipantSH::Mark   XPC_WN_Helper_Mark   js_Mark   MarkGCThingChildren   MarkGCThingChildren   MarkGCThingChildren   js_MarkGCThing   js_GC   js_ForceGC   NS_ProcessNextEvent_P   nsBaseAppShell::Run   0x00392aac
0xccccc3c0

Of course, maybe this is not Mozilla's fault, and is Firebug doing something wrong.

Comment 1

[Martijn Wargers (dead)]

Attachment: testcase

This testcase crashes for me when I have Firebug installed.

Comment 2

[timeless]

firebug generally can't do things wrong, sure it's possible for chrome code to maliciously trigger a crash, but in almost all cases it's either jsd_xpc, xpconnect or jsd's fault. sometimes it's just dom or spidermonkey's fault.

Comment 3

[Martijn Wargers (dead)]

Attachment: code from firebug that causes the crash

This is basically the code from firebug that causes the crash. I tried to make a testcase out of it, but failed.
I know this causes it, because, when I uncomment 
     if (this.showXMLHttpRequests)
            this.attachXMLHttpSpy(context, win);
in firebug.js, then the crash doesn't happen.

Comment 4

[Adam Guthrie]

*** Bug 342509 has been marked as a duplicate of this bug. ***

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This works for me using a 1.8 branch Linux debug+opt build.  Or is there something I'm supposed to do other than install firebug, restart, and load the testcase?

Comment 6

[Martijn Wargers (dead)]

No, that's all you should do to get this bug. I can still reproduce this on current trunk windows builds. I haven't been able yet to get a good stacktrace in a debug build.

Comment 7

[Sylvain Pasche]

(In reply to comment #5)
> This works for me using a 1.8 branch Linux debug+opt build.  Or is there
> something I'm supposed to do other than install firebug, restart, and load the
> testcase?
> 

Be sure to have "Show XmlHttpRequests" checkbox activated in the firebug's Options menu. It is not checked by default, and without it checked, there's no crash.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to comment #7)
> Be sure to have "Show XmlHttpRequests" checkbox activated in the firebug's
> Options menu. It is not checked by default, and without it checked, there's no
> crash.

I don't see any such option inside Tools -> Firebug.  Is there some other "Options menu"?

Comment 9

[Martijn Wargers (dead)]

(In reply to comment #8)
> I don't see any such option inside Tools -> Firebug.  Is there some other
> "Options menu"?

You can enable xmlhttprequests logging here (but I thought it was enabled by default), I've made an image of it:
http://wargers.org/firebug.gif

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I got it to crash in a debug build, but not while running under valgrind.

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

This turns out to be a pretty trivial problem.  In nsXMLHttpRequest::RemoveEventListener, I leak the ListenerHolder (a typedef to nsMarkedJSFunctionHolder<nsIDOMEventListener>).  (I can't use nsTArray<ListenerHolder> because I need pointer identity on the ListenerHolder objects.)  Leaking the ListenerHolder means its destructor doesn't run, and its destructor cleans up a pointer to the nsXMLHttpRequest object.

Comment 12

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 231648 [details] [diff] [review]
patch

looks good, r/sr=sicking

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked in to trunk.

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked in to MOZILLA_1_8_BRANCH.

Comment 15

[Martijn Wargers (dead)]

Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b1) Gecko/20060802 BonEcho/2.0b1
using the testcase and with Firebug installed.