Closed
Bug 527567
Opened 15 years ago
Closed 15 years ago
Crash @ [@nsCOMPtr_base::assign_from_qi(nsQueryInterface, nsID const&) ]
Categories
(Core :: General, defect)
Tracking
()
RESOLVED
DUPLICATE
of bug 528134
People
(Reporter: cbook, Unassigned)
References
(Blocks 1 open bug)
Details
(Keywords: crash, Whiteboard: [crashkill][sg:dupe 528134])
Crash Data
Currently #40 of the TopCrash Stats http://crash-stats.mozilla.com/report/list?product=Firefox&version=Firefox%3A3.6b1&query_search=signature&query_type=exact&query=&date=&range_value=1&range_unit=weeks&do_query=1&signature=nsCOMPtr_base%3A%3Aassign_from_qi%28nsQueryInterface%2C%20nsID%20const%26%29 other then "Google PageSpeed plugin crashed when analysing a page" no other steps to reproduce mentioned - investigating
Flags: blocking1.9.2?
Reporter | ||
Updated•15 years ago
|
Whiteboard: [crashkill]
Comment 1•15 years ago
|
||
somne of these like the report with the comment about google page speed are frame poisoning bugs. http://crash-stats.mozilla.com/report/index/91c44378-9884-4ba5-8e87-95a6c2091109 Frame Module Signature [Expand] Source 0 xul.dll nsCOMPtr_base::assign_from_qi obj-firefox/xpcom/build/nsCOMPtr.cpp:96 1 xul.dll nsCOMPtr<nsICSSStyleRule>::operator= obj-firefox/dist/include/nsCOMPtr.h:658 2 xul.dll inDOMUtils::GetCSSStyleRules layout/inspector/src/inDOMUtils.cpp:181 3 xul.dll NS_InvokeByIndex_P xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:101 4 xul.dll XPCWrappedNative::CallMethod js/src/xpconnect/src/xpcwrappednative.cpp:2710 sort base on crash address to see more 0xfffffffff0dea7ff
Blocks: PoisonFrameCrash
Group: core-security
Comment 2•15 years ago
|
||
These reports are badly aggregated -- nsCOMPtr_base::assign_from_qi is called from a handful of nsCOMPtr methods which in turn are called from, like, *everywhere*. Can we have the crash analysis peel away all frames that are nsCOMPtr or nsCOMPtr_base methods, and aggregate based on their caller? (inDOMUtils::GetCSSStyleRules in the above example) I didn't look at all of the crashes at f0dea7ff, but a lot of them have inDOMUtils::GetCSSStyleRules as the third stack frame. The crash is on line 181 of this loop: hg@1 178 nsCOMPtr<nsICSSStyleRule> cssRule; hg@1 179 nsCOMPtr<nsIDOMCSSRule> domRule; dbaron@31313 180 for ( ; !ruleNode->IsRoot(); ruleNode = ruleNode->GetParent()) { dbaron@31313 181 cssRule = do_QueryInterface(ruleNode->GetRule()); hg@1 182 if (cssRule) { hg@1 183 cssRule->GetDOMRule(getter_AddRefs(domRule)); hg@1 184 if (domRule) hg@1 185 rules->InsertElementAt(domRule, 0); hg@1 186 } hg@1 187 } Rule nodes are frame arena objects; nsICSSStyleRule objects aren't. It looks to me like a rule node was destroyed while still reachable.
Comment 3•15 years ago
|
||
(In reply to comment #2) > Can we have the crash analysis peel away all frames that are > nsCOMPtr or nsCOMPtr_base methods, and aggregate based on their caller? > (inDOMUtils::GetCSSStyleRules in the above example) Yes, file a bug against Webtools:Socorro like bug 504498. Which then apparently results in a second IT bug being filed but since that one's hidden I can't tell you what details you'd need to cut out the middleman.
Comment 4•15 years ago
|
||
I reopened bug 504498 for the crash analysis adjustment, since Lars seemed to prefer that.
Comment 5•15 years ago
|
||
> It looks to me like a rule node was destroyed while still reachable.
There's actually an obvious way this can happen, I think. inDOMUtils::GetRuleNodeForContent is clearly buggy: it returns the rulenode, but not the style context. If the element is in a display:none subtree and if the destructor for the sContext local triggers rulenode gc, this can easily hand back a dead object, no? Let's get a bug filed on that, blocking this one?
Comment 6•15 years ago
|
||
Filed bug 528134.
Comment 7•15 years ago
|
||
Is this turning into a meta bug with dependents? Not sure what sg severity to assign it.
Whiteboard: [crashkill] → [crashkill][sg:investigate]
Comment 8•15 years ago
|
||
> Is this turning into a meta bug with dependents? It sort of has to, due to comment 2, no? Crash in this function could be all sorts of stuff depending on what else is on the stack.
Comment 9•15 years ago
|
||
I hit this crash when exiting Firefox on Windows 7 - http://crash-stats.mozilla.com/report/index/bp-b1a37de0-e9aa-4c96-9761-2c3352091117 is my report but I have not been able to reproduce.
Comment 10•15 years ago
|
||
Filed bug 529452 on Marcia's crash.
Comment 11•15 years ago
|
||
Don't block on metas, sorry; please nominate the crashes as we discover them.
Flags: blocking1.9.2? → blocking1.9.2-
Comment 12•15 years ago
|
||
once the skip list change in Bug 504498 goes in we should be able to analysis of nsCOMPtr_base::assign_from_qi that have addresses with 0xfffffffff0dea7ff, right? I marked bug 504498 as blocking. when that is done, we should have a specific bug for nsCOMPtr_base::assign_from_qi that have addresses with 0xfffffffff0dea7ff or we could use this one, any preferences?
Comment 13•15 years ago
|
||
(In reply to comment #12) > once the skip list change in Bug 504498 goes in we should be able to analysis > of nsCOMPtr_base::assign_from_qi that have addresses with 0xfffffffff0dea7ff, > right? I don't understand this statement. The requested skip list change should remove *all* instances of nsCOMPtr[_base]::<anything> from the crash statistics - instead they'll show up under the callers.
Comment 14•15 years ago
|
||
ok, I'll try and rephrase in the case of comment 2 we should see the signature change from nsCOMPtr_base::assign_from_qi(nsQueryInterface, nsID const&) with crash address w/ crash address 0xfffffffff0dea7ff change to a signature that might look like nsCOMPtr<nsICSSStyleRule>::operator= our options are to change the title of this bug and use it to track the bug comment 2, or to file a new one, correct? we don't have a clean way of handling changes caused by updates to the skip list. maybe it's just better to start with new clean bugs.
Comment 15•15 years ago
|
||
We already have a separate bug tracking the bug of comment 2. It's blocking this bug. See comment 6.
Comment 16•15 years ago
|
||
I think the thing to do is file new bugs for each new signature, once we have the new signatures. They could block this bug, but I'd be happy with closing this one and tracking them as dependencies of bug 526587 instead.
With the requested skip-list change, I think the signature for http://crash-stats.mozilla.com/report/index/91c44378-9884-4ba5-8e87-95a6c2091109 should show up as: nsCOMPtr_base::assign_from_qi | nsCOMPtr<nsICSSStyleRule>::operator= | inDOMUtils::GetCSSStyleRules That is, things in the skip list get concatenated together until something not in the skip list is found. (But numeric addresses are just skipped completely.)
Comment 18•15 years ago
|
||
I'm changing this back to a nom for 1.9.2 because we need to get a skiplist change in asap to see if this is a regression from 3.5. Right now, we can't tell. Jonas to file a new bug and add this as a dependency.
Flags: blocking1.9.2- → blocking1.9.2?
Comment 19•15 years ago
|
||
(In reply to comment #18) > Jonas to file a new bug and add this as a dependency. (bug 530684)
Now that the skiplist changes have landed, we're getting the reports split up. The biggest chunk of Firefox3.6b3 crash reports at this signature are the ones from doGetObjectPrincipal; I think that's the way bug 521844 shows up on Mac (it shows up as doGetObjectPrincipal itself on Windows). The second biggest chunk (and the biggest chunk on Windows) are bug 528134; those are the ones with a frame poisoning signature.
Comment 21•15 years ago
|
||
So does that make this a regression or not? Still doesn't feel like this specific bug is something that we can block on. Bug 528134 was resolved as a duplicate of bug 519719 which is a blocker, and bug 521844 is fixed on trunk and mozilla1.9.2, so removing the nomination again.
Flags: blocking1.9.2? → blocking1.9.2+
Updated•15 years ago
|
Flags: blocking1.9.2-
I think we should just file separate bugs for each remaining problem. Given that most of the technical discussion in this bug was about one of the problems (things about google page speed, nsICSSStyleRule, etc.), though, I'll mark it as a duplicate of that one.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → DUPLICATE
Updated•15 years ago
|
Whiteboard: [crashkill][sg:investigate] → [crashkill][sg:dupe 528134]
Updated•14 years ago
|
Group: core-security
Assignee | ||
Updated•13 years ago
|
Crash Signature: [@nsCOMPtr_base::assign_from_qi(nsQueryInterface, nsID const&) ]
You need to log in
before you can comment on or make changes to this bug.
Description
•