Crash when loading any XSLT file from most websites with Assertion failure: mDocGroup->MatchesKey(docGroupKey), at nsDocument.cpp:2985

RESOLVED FIXED in Firefox 52

Status

()

defect
--
critical
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: cbook, Assigned: Ehsan)

Tracking

(Blocks 1 bug, {assertion, crash, regression})

unspecified
mozilla54
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox52+ fixed, firefox53+ fixed, firefox54+ fixed)

Details

(crash signature, )

Attachments

(4 attachments)

Posted file bughunter stack
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 ?
Flags: needinfo?(wmccloskey)
Flags: needinfo?(mstange)
Flags: needinfo?(wmccloskey)
Ehsan, Michael, do you guys have any advice? I've never even heard of these functions.
Flags: needinfo?(mstange)
Flags: needinfo?(michael)
Flags: needinfo?(ehsan)
I'll debug.
Flags: needinfo?(ehsan)
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
Flags: needinfo?(michael)
See Also: → 1334281
Depends on: 1334320
No longer depends on: 1334320
See Also: → 1334320
See Also: → 1334322
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.
Blocks: 1303196
Crash Signature: [@ nsDocument::SetScopeObject]
Keywords: regression
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.
See Also: 1334320, 1334322
Blocks: 1334281
See Also: 1334281
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.
Attachment #8831158 - Flags: approval-mozilla-beta?
Attachment #8831158 - Flags: approval-mozilla-aurora?
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+
Attachment #8831158 - Flags: approval-mozilla-beta?
Attachment #8831158 - Flags: approval-mozilla-beta+
Attachment #8831158 - Flags: approval-mozilla-aurora?
Attachment #8831158 - Flags: approval-mozilla-aurora+
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
Flags: needinfo?(ehsan)
backed out from aurora for bustage still https://treeherder.mozilla.org/logviewer.html#?job_id=72993930&repo=mozilla-aurora
Flags: needinfo?(ehsan)
Peter, do you think you'll get a chance to review this?  If not can you please suggest someone else?
Flags: needinfo?(peterv)
Attachment #8831156 - Flags: review?(peterv) → review+
Thanks for the review!
Flags: needinfo?(peterv)
Pushed by eakhgari@mozilla.com:
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
https://hg.mozilla.org/mozilla-central/rev/28974f2c413c
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.