Using the browser toolbox I was debugging a http-on-modify-request observer and had a nsiHttpChannel in the local scope. Upon inspecting it and trying to drill down on the loadGroup property of the channel the browser crashed. crash id: bp-c1c8e520-9b1a-4c3d-a2b0-b622e2150825
Do you have STR for the crash?
Created attachment 8652097 [details] greasemonkey-2015.08.25.beta.xpi see if this works for you: - clean profile - e10s off - addon signature verification off - install custom greasemonkey build (attached) - open browser toolbox - set a breakpoint on refererSetter.js line 33 - open a page to trigger the observer - select channel from the local scope in the variables view - try to open the "loadGroup" property That's on nightly Build ID 20150821030204
(In reply to The 8472 from comment #2) > - set a breakpoint on refererSetter.js line 33 I don't see this js script in the debugger.
did you restart after installing the xpi? did you use the browser toolbox (not the inline debugger)?
Done, I got the crash: https://crash-stats.mozilla.com/report/index/abf35e9f-6fbb-41e7-bbd3-8f99d2150825
Looks like XPConnect on the stack, perhaps that group may know more.
Component: Developer Tools: Debugger → XPConnect
Product: Firefox → Core
It regressed: good=2015-08-05 bad=2015-08-06 http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f3b757156f69&tochange=07befc6f54e7 Suspected bug: Andrew McCreight — Bug 1190436 - Part 2: Use an early return in XPCConvert::JSObject2NativeInterface. r=gabor Andrew McCreight — Bug 1190436 - Part 1: Use more smart pointers in XPConnect. r=gabor
Severity: normal → critical
Status: UNCONFIRMED → NEW
Crash Signature: [@ nsJSID::NewID(nsID const&) ]
status-firefox41: --- → unaffected
status-firefox42: --- → affected
status-firefox43: --- → affected
tracking-firefox42: --- → ?
tracking-firefox43: --- → ?
Ever confirmed: true
Keywords: crash, regression
Summary: Debugging/inspecting a nsiHttpChannel crashes browser → Debugging/inspecting a nsiHttpChannel crashes browser in nsJSID::NewID(nsID const&)
Version: Trunk → 42 Branch
Assignee: nobody → continuation
Any chance you could try bisecting across inbound using mozregression to narrow the range a bit? Thanks. It isn't immediately clear to me how my patches could have caused this, though I do agree they are the most suspect in that range.
m-i: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=579d50cc0ca7&tochange=91a3e2205388 I see: Andrew McCreight — Bug 1155773 - Remove gotos from XPCConvert::NativeArray2JS(). r=bholley
tracking-firefox42: ? → +
tracking-firefox43: ? → +
I can reproduce this locally. I'm still not sure what is the matter here. It looks like the nsID we're getting in XPCConvert::NativeData2JS() is bogus. I reverted bug 1155773 and bug 1190436, but that did not fix the crash.
I did my own mozregression which pointed to bug 1127618 (which matches up with the regression range from comment 9, but not the prior ones): https://hg.mozilla.org/integration/mozilla-inbound/rev/69f2a0f7b8ad400e807a2c57424585c2b4ff9d31 I was able to confirm the regression using local builds. That patch involves networking, and the reported problem seems to involve nsIHttpChannel so there's at least some relation. Could you look into this regression, Nick? Thanks.
Lest anyone think I haven't noticed this - I have. Unfortunately, while I can reproduce the crash, I'm currently no closer to finding out why the crash is occurring. My one thought so far (and this is a long shot!) is that the code to serialize an "attribute nsID" into script is somehow incorrect - my use added in bug 1127618 is the *only* use of that, so I have no others to compare with (any other script-accessible nsIDs are through an "attribute nsIDPtr"). I'm continuing to dig in to see if I can narrow down the problem more.
Um... Why are you using "attribute nsID"? That's not supported from script at all, and will cause you bad times and corrupted memory. See the comment at http://mxr.mozilla.org/mozilla-central/source/xpcom/base/nsrootidl.idl#57
Oh, and the fact that xpidl didn't barf on that is a bug, at least assuming I believe the last sentence of that comment...
Well, that would certainly explain the crash :) I don't see any reason for this to be accessible via script, so I'll just [notxpcom] it. (FWIW, I wanted 'attribute nsID' because I didn't want to have to bother manually freeing memory for the nsIDs.) And yes, if that comment is supposed to be correct (aren't they all?! :P) then this does sound like an xpidl bug, as well.
> I don't see any reason for this to be accessible via script Ah, good. Then let's notxpcom by all means. And if that fixes the crash, please file another bug on xpidl.
Created attachment 8662014 [details] [diff] [review] patch So I actually went with 'noscript' instead of 'notxpcom' (xpidl seemed to barf on 'notxpcom' by itself - maybe because the interface is labeled 'scriptable'?) but it has the effect we want. Interesting side note, I appear to have made all other instances of an 'attribute nsID' that I added to scriptable interfaces 'noscript' in the original patch, so obviously I at some point knew that was the right thing to do. I just didn't do it for nsILoadGroup, for some reason.
Attachment #8662014 - Flags: review?(mcmanus)
(In reply to Boris Zbarsky [:bz] from comment #16) > > I don't see any reason for this to be accessible via script > > Ah, good. Then let's notxpcom by all means. And if that fixes the crash, > please file another bug on xpidl. Filed bug 1205419
Attachment #8662014 - Flags: review?(mcmanus) → review+
dont forget to send it to 42
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox43: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Comment on attachment 8662014 [details] [diff] [review] patch Approval Request Comment [Feature/regressing bug #]: 1127618 [User impact if declined]: crashes when inspecting certain objects in browser toolbox [Describe test coverage new/current, TreeHerder]: on m-c [Risks and why]: low [String/UUID change made/needed]: nsILoadGroup UUID changed to make an attribute noscript. The UUID for this already changed in gecko 42, anyway. Note: this may need to be approved for beta instead, if this doesn't get approved and landed on aurora before the next uplift.
Attachment #8662014 - Flags: approval-mozilla-aurora?
Comment on attachment 8662014 [details] [diff] [review] patch Fix a crash, taking it.
Attachment #8662014 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8662014 [details] [diff] [review] patch (Copied from comment 23 - this didn't get approved in time for me to get this into aurora 42, so now it has to go into beta 42) Approval Request Comment [Feature/regressing bug #]: 1127618 [User impact if declined]: crashes when inspecting certain objects in browser toolbox [Describe test coverage new/current, TreeHerder]: on m-c and m-a [Risks and why]: low [String/UUID change made/needed]: nsILoadGroup UUID changed to make an attribute noscript. The UUID for this already changed in gecko 42, anyway.
when applying this to beta i get: remote: *** IDL file netwerk/base/nsILoadGroup.idl altered in this changeset*** remote: remote: Changes to IDL files in this repo require you to provide binary change approval in your top comment in the form of ba=... (or, more accurately, ba\S*=...) remote: This is to ensure that UUID changes (or method changes missing corresponding UUID change) are caught early, before release. is this expected ?
Well, yes and no. The patch was expected to land in aurora and not in beta2... Jorge, are you ok with this IDL change? Thanks
Flags: needinfo?(sledru) → needinfo?(jorge)
Looks good to me.
status-firefox42: affected → fixed
You need to log in before you can comment on or make changes to this bug.