crash in nsGenericHTMLFrameElement EXCEPTION_ACCESS_VIOLATION_READ

RESOLVED FIXED in Firefox 42

Status

()

Core
DOM
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: marvinhk, Assigned: bz)

Tracking

40 Branch
mozilla42
Points:
---

Firefox Tracking Flags

(firefox42 fixed)

Details

(crash signature)

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:40.0) Gecko/20100101 Firefox/40.0
Build ID: 20150709163524

Steps to reproduce:

1. pref: don't load tab until selected
2. start firefox with session restore (6 windows with around 10 tabs each)
3. for each windows, right click on the 1st (loaded) tab and "Reload All Tabs"




Actual results:

crash occurs when the tabs are refreshing

note: if the number of windows to be restored increased, it becomes a start-up crash



Expected results:

tabs are reloaded succesfully without crash
(Reporter)

Updated

3 years ago
Crash Signature: [@ nsGenericHTMLFrameElement::GetContentDocument() | mozilla::dom::HTMLIFrameElementBinding::get_contentDocument]
Component: General → DOM
(Reporter)

Updated

3 years ago
Crash Signature: [@ nsGenericHTMLFrameElement::GetContentDocument() | mozilla::dom::HTMLIFrameElementBinding::get_contentDocument] → [@ nsGenericHTMLFrameElement::GetContentDocument() ]
Do you have a link from about:crashes?
Flags: needinfo?(marvinhk)
(Reporter)

Comment 2

3 years ago
crash report is here <https://crash-stats.mozilla.com/report/index/1ba0691d-815d-4f35-b241-853d22150714>
Flags: needinfo?(marvinhk)
Thanks, and sorry for the lag on my end here....

This looks like a null-deref on the "doc->NodePrincipal()" line.  That's a bit confusing to me; I would have expected GetContentWindow to return null if we no longer have a window with a document...

In any case, I guess the right thing here is to add a null-check.

Also, I expect there's an extension involved, since the get is coming from a scripted content policy.  So it's possible that the extension will end up with exceptions (instead of this crash) if it doesn't null-check the .contentDocument it's getting.
Created attachment 8640034 [details] [diff] [review]
Null-check the document we get from our contentWindow in the contentDocument getter, because apparently it can end up null
Attachment #8640034 - Flags: review?(bkelly)
Assignee: nobody → bzbarsky
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

Comment 5

3 years ago
Comment on attachment 8640034 [details] [diff] [review]
Null-check the document we get from our contentWindow in the contentDocument getter, because apparently it can end up null

Review of attachment 8640034 [details] [diff] [review]:
-----------------------------------------------------------------

This looks reasonable, but I can't help but wonder if we should be setting mDoc here:

  http://mxr.mozilla.org/mozilla-central/source/dom/base/nsGlobalWindow.cpp#3440

That comment looks bogus.  I can't find where docshell calls SetNewDocument().
Attachment #8640034 - Flags: review?(bkelly) → review+
> I can't find where docshell calls SetNewDocument().

nsDocShell::GetDocument calls EnsureContentViewer() (carefully hidden in the NS_ENSURE_SUCCESS macro) which calls CreateAboutBlankContentViewer which ends up doing various stuff that lands in nsDocumentViewer::InitInternal which calls SetNewDocument.
(Reporter)

Comment 7

3 years ago
just additional information: firefox lags a lot and crashes in nsGenericHTMLFrameElement starts happening since ff40. it is possible to be a side effect of omtc being enabled by default in ff40. perf off otmc at the moment to see if this continues to happen.
https://hg.mozilla.org/mozilla-central/rev/87121bfe4be9
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox42: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
(Reporter)

Comment 10

3 years ago
seems the issue is still there in ff41 <https://crash-stats.mozilla.com/report/index/bf749a0a-a809-4beb-a941-9bab62150826>
You need to log in before you can comment on or make changes to this bug.