140.80 KB, text/plain
3.65 KB, text/plain
1.57 KB, patch
|Details | Diff | Splinter Review|
Remove the release assertion for docgroup keys matching until we fix the code to adhere to this (branch fix)
1.19 KB, patch
|Details | Diff | Splinter Review|
Assertion failure: mDocGroup->MatchesKey(docGroupKey), at c:/builds/moz2_slave/m-cen-w32-d-000000000000000000/build/src/dom/base/nsDocument.cpp:2985 found by bughunter reproduced with latest mozilla debug trunk tinderbox windows build on win7 Bughunter data show that Linux and Windows are affected as well as all branches like aurora,beta and trunk Steps to reproduce: -> Load https://www.checkcolorado.com/caseDisplay.php?case_id=2005T104174%28County%29&county_code=LAR --> Crash on load also crashes opt builds like https://crash-stats.mozilla.com/report/index/d8d90bf7-7b92-4712-ab44-6ea162170126
[Tracking Requested - why for this release] crashes opt and debug builds trunk -> beta bill, markus: can you take a look ?
Ehsan, Michael, do you guys have any advice? I've never even heard of these functions.
The case at hand here is an XSLT document which uses the text output method. We set up the DocGroup here <http://searchfox.org/mozilla-central/rev/7da3c9dcf467964f2fb82f3a4c63972ee79bf696/dom/xslt/xslt/txMozillaTextOutput.cpp#152> but at that point we have the null principal set on mDocument so we get a DocGroup with an empty key, and then we get to here <http://searchfox.org/mozilla-central/rev/7da3c9dcf467964f2fb82f3a4c63972ee79bf696/dom/xslt/xslt/txMozillaTextOutput.cpp#157> where we call ResetToURI which causes us to get the correct principal under this call stack: frame #1: 0x000000011182783f XUL`nsDocument::SetPrincipal(this=0x000000010bbe3000, aNewPrincipal=0x000000010becfce0) + 335 at nsDocument.cpp:2858 frame #2: 0x0000000111822012 XUL`nsDocument::ResetToURI(this=0x000000010bbe3000, aURI=0x000000010bab3020, aLoadGroup=0x000000010771f020, aPrincipal=0x000000010becfce0) + 1442 at nsDocument.cpp:2092 frame #3: 0x0000000113bb34da XUL`mozilla::dom::XMLDocument::ResetToURI(this=0x000000010bbe3000, aURI=0x000000010bab3020, aLoadGroup=0x000000010771f020, aPrincipal=0x000000010becfce0) + 138 at XMLDocument.cpp:274 frame #4: 0x000000011182188e XUL`nsDocument::Reset(this=0x000000010bbe3000, aChannel=0x000000010bbb5800, aLoadGroup=0x000000010771f020) + 334 at nsDocument.cpp:1976 frame #5: 0x0000000113bb3440 XUL`mozilla::dom::XMLDocument::Reset(this=0x000000010bbe3000, aChannel=0x000000010bbb5800, aLoadGroup=0x000000010771f020) + 48 at XMLDocument.cpp:261 frame #6: 0x0000000113bc9195 XUL`URIUtils::ResetWithSource(aNewDoc=0x000000010bbe3000, aSourceNode=0x000000010f347000) + 565 at txURIUtils.cpp:77 frame #7: 0x0000000113c0fd17 XUL`txMozillaTextOutput::createResultDocument(this=0x000000010a70f100, aSourceDocument=0x000000010f347000, aLoadedAsData=false) + 967 at txMozillaTextOutput.cpp:157 frame #8: 0x0000000113c1756c XUL`txToDocHandlerFactory::createHandlerWith(this=0x00007fff58c778f0, aFormat=0x0000000128257ee0, aHandler=0x00007fff58c77828) + 588 at txMozillaXSLTProcessor.cpp:118 frame #9: 0x0000000113bfd2b6 XUL`txExecutionState::init(this=0x00007fff58c77968, aNode=0x0000000128194da0, aGlobalParams=0x000000010be60c50) + 198 at txExecutionState.cpp:116 At this point we get the correct principal and compute a non-empty key, and the assertion fails because we have already set up DocGroup. What's even more interesting is that we have managed ~15 years since this code landed in bug 155578 without a single test case for this. :(
Assignee: nobody → ehsan
Component: DOM: Content Processes → XSLT
Note that due to bug 1334322, this bug currently won't get triggered when running reftests/crashtest, and due to bug 1334320 it also won't get triggered from reftests/crashtests that have the HTTP annotation in the manifest (since they're served from http://localhost). Otherwise this would crash us for any XSLT document being loaded. :( See for example bp-08817b04-8f18-426f-9e2f-82e2d2170126. I'll fix those bugs first to see if my hypothesis above is correct. But this fix should be backported to 52.
Crash Signature: [@ nsDocument::SetScopeObject]
Summary: Assertion failure: mDocGroup->MatchesKey(docGroupKey), at nsDocument.cpp:2985 → Crash when loading any XSLT file from most websites with Assertion failure: mDocGroup->MatchesKey(docGroupKey), at nsDocument.cpp:2985
(Thankfully release is unaffected.)
I just confirmed that fixing bug 1334281 will make our existing crashtests catch this.
Tracking for 52 and up, per comment 6.
Because of bug 1334281 comment 5, I think a good branch fix for this would be removing the assertion.
Attachment #8831158 - Flags: review?(wmccloskey) → review+
Comment on attachment 8831158 [details] [diff] [review] Remove the release assertion for docgroup keys matching until we fix the code to adhere to this (branch fix) Approval Request Comment [Feature/Bug causing the regression]: Bug 1303196. [User impact if declined]: Crash when visiting most web-hosted XSLT pages. [Is this code covered by automated tests?]: This patch doesn't need tests. [Has the fix been verified in Nightly?]: No, this is a branch specific patch disabling an assertion. We'll land a real fix on trunk. [Needs manual test from QE? If yes, steps to reproduce]: Np. [List of other uplifts needed for the feature/fix]: None. [Is the change risky?]: No. [Why is the change risky/not risky?]: See above. [String changes made/needed]: None.
Comment on attachment 8831158 [details] [diff] [review] Remove the release assertion for docgroup keys matching until we fix the code to adhere to this (branch fix) remove an assertion to fix crash, aurora53+, beta52+
has problems to apply to beta: grafting 386896:0411363a0dc7 "Bug 1334047 - Remove the release assertion for docgroup keys matching until we fix the code to adhere to this (branch fix); r=billm a=jcristau" merging dom/base/nsDocument.cpp warning: conflicts while merging dom/base/nsDocument.cpp! (edit, then use 'hg resolve --mark') abort: unresolved conflicts, can't continue (use 'hg resolve' and 'hg graft --continue') Tomcats-MacBook-Pro-300:mozilla-central Tomcat
backed out also from aurora for causing bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=72985697&repo=mozilla-aurora
backed out from aurora for bustage still https://treeherder.mozilla.org/logviewer.html#?job_id=72993930&repo=mozilla-aurora
Peter, do you think you'll get a chance to review this? If not can you please suggest someone else?
Attachment #8831156 - Flags: review?(peterv) → review+
Thanks for the review!
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/28974f2c413c Only set the script handling object for XSLT result documents after setting up the principal; r=peterv
You need to log in before you can comment on or make changes to this bug.