1808177 - Crash in [@ nsINode::OwnerDoc]

Status: RESOLVED FIXED

(Core :: DOM: File, defect, P1)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Crash signature new with Firefox v108, observed across operating systems. 280 crash reports recorded for the last week.

Regression from bug 1778565?

Crash report: https://crash-stats.mozilla.org/report/index/eaea85cf-747c-4a35-8a30-67eef0230102

Reason: EXCEPTION_ACCESS_VIOLATION_READ

Top 10 frames of crashing thread:

0  xul.dll  RefPtr<mozilla::dom::NodeInfo>::get const  mfbt/RefPtr.h:286
0  xul.dll  RefPtr<mozilla::dom::NodeInfo>::operator-> const  mfbt/RefPtr.h:316
0  xul.dll  nsINode::OwnerDoc const  dom/base/nsINode.h:686
0  xul.dll  mozilla::HTMLEditor::BlobReader::OnError  editor/libeditor/HTMLEditorDataTransfer.cpp:1729
1  xul.dll  mozilla::SlurpBlobEventListener::HandleEvent  editor/libeditor/HTMLEditorDataTransfer.cpp:1792
2  xul.dll  mozilla::EventListenerManager::HandleEventSubType  dom/events/EventListenerManager.cpp:1316
2  xul.dll  mozilla::EventListenerManager::HandleEventInternal  dom/events/EventListenerManager.cpp:1506
2  xul.dll  mozilla::EventListenerManager::HandleEvent  dom/events/EventListenerManager.h:395
2  xul.dll  mozilla::EventTargetChainItem::HandleEvent  dom/events/EventDispatcher.cpp:348
3  xul.dll  mozilla::EventTargetChainItem::HandleEventTargetChain  dom/events/EventDispatcher.cpp:550

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The bug is linked to a topcrash signature, which matches the following criteria:

• Top 10 content process crashes on beta
• Top 10 content process crashes on release

:edenchuang, could you consider increasing the severity of this top-crash bug?

For more information, please visit auto_nag documentation.

Comment 2

[Tom S [:evilpie]]

Attachment: Bug 1808177 - mPointToInsert.GetContainer() in BlobReader::OnError could be nullptr. r?masayuki

Comment 3

[Dianna Smith [:diannaS]]

:evilpies given the crash volume, any way to get this landed today so it can be included in the dot release? Depending on your assessment on risk level, could you nominate this for beta/release?

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/96e1597780fa
mPointToInsert.GetContainer() in BlobReader::OnError could be nullptr. r=masayuki

Comment 5

[Tom S [:evilpie]]

Comment on attachment 9310479 [details]
Bug 1808177 - mPointToInsert.GetContainer() in BlobReader::OnError could be nullptr. r?masayuki

Beta/Release Uplift Approval Request

• User impact if declined: Crashes
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Not really clear how to test this, but the currently code is clearly problematic, because we know that the pointer might be null.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The code change is tiny and should be totally unproblematic.
• String changes made/needed:
• Is Android affected?: Yes

Comment 6

[Dianna Smith [:diannaS]]

Comment on attachment 9310479 [details]
Bug 1808177 - mPointToInsert.GetContainer() in BlobReader::OnError could be nullptr. r?masayuki

Approved for 108.0.2

Comment 7

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/96e1597780fa

Comment 8

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-release/rev/a486bbf61993

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9310479 [details]
Bug 1808177 - mPointToInsert.GetContainer() in BlobReader::OnError could be nullptr. r?masayuki

Approved for 109.0b9.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Is there any chance we could create an automated test for this?

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/8e6b56b8372f

Comment 12

[Daniel Bodea [:danibodea]]

I've attempted to reproduce this without results. Considering the crash report comments, I've attempted to compose emails and paste any kind of data that I could think of it into them, repeating the action several times. I am not sure how this reproduces, but I could not get the hang of it.

Furthermore, I've reproduced a crash with another crash signature, while attempting to reproduce this.
https://crash-stats.mozilla.org/report/index/bc9d65d5-b6ee-4370-92f5-a38270230105

Comment 13

[Jens Stutte [:jstutte]]

(In reply to Bodea Daniel [:danibodea] from comment #12)

Furthermore, I've reproduced a crash with another crash signature, while attempting to reproduce this.
https://crash-stats.mozilla.org/report/index/bc9d65d5-b6ee-4370-92f5-a38270230105

This seems to be just an OOM - did you paste by chance a very long text here?

Comment 14

[Daniel Bodea [:danibodea]]

Yes, I have pasted all kinds of data in a mail draft, but I wouldn't say it was a very long one. Furthermore, I have to say that the tab appeared to be fine after the paste, only after turning my eyes from it, did the tab crash. Do you want me to attempt to reproduce it again and log it? or mention it in some other OOM crash bug? Thanks.

Comment 15

[Jens Stutte [:jstutte]]

I am unsere if this is a good use of your time, in any case it would be a different bug. If you try this, it could be helpful to start the profiler before attempting to reproduce to see if memory is growing unexpectedly high.

Comment 16

[Tom S [:evilpie]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #10)

Is there any chance we could create an automated test for this?

Actually not that simple. I think we need to create a Blob that causes an error when read using the FileReader. I think that means we have to create some kind of File that is backed by an inaccessible real file? I haven't been able to find a similar test for the DOM FileReader API.

Comment 17

[Olli Pettay [:smaug][[e-mail]](vacation until Nov 16)]

Yeah, no idea how to create a test.