Closed Bug 1530519 Opened 7 months ago Closed 7 months ago

Crash with "unknown union type" in PContentParent::SendInvokeDragSession

Categories

(Core :: DOM: Core & HTML, defect, P2, critical)

defect

Tracking

()

RESOLVED FIXED
mozilla67
Tracking Status
firefox-esr60 --- wontfix
firefox65 --- wontfix
firefox66 --- fixed
firefox67 --- fixed

People

(Reporter: marcia, Assigned: evilpie)

Details

(Keywords: crash, regression, regressionwindow-wanted)

Crash Data

Attachments

(1 file)

This bug is for crash report bp-a8a39ee4-224a-4788-bc65-28f8c0190225.

Seen while looking at Mac 65 crash stats: https://bit.ly/2BRdYOk. This crash appears to be new in 65 and spans all versions of Mac OS. Many of the comments mention copy and pasting text. Perhaps a signature change, not sure since it was not seen in 65 beta.

Here are some comments:

  • Podcast concluded. "Beachball" appeared. FF crashed. Recent installation of Bitwarden.
  • I have been copying text from a pdf into a WordPress site and these last few times, it keeps kicking me out of FireFox each time.
  • Tried to copy/paste from Excel into Google Sheets.
  • i was dragging a picture into google slides and the program froze and crashed
Top 10 frames of crashing thread:

0 XUL mozilla::ipc::FatalError ipc/glue/ProtocolUtils.cpp:259
1 XUL .str.1.llvm.2376781057090901720 
2 XUL void mozilla::ipc::WriteIPDLParam<nsTArray<mozilla::dom::IPCDataTransferItem> const&> ipc/glue/IPDLParamTraits.h:145
3 XUL void mozilla::ipc::WriteIPDLParam<nsTArray<mozilla::dom::IPCDataTransfer> const&> ipc/glue/IPDLParamTraits.h:145
4 XUL mozilla::dom::PContentParent::SendInvokeDragSession ipc/ipdl/PContentParent.cpp:2222
5 XUL mozilla::dom::ContentParent::MaybeInvokeDragSession dom/ipc/ContentParent.cpp:4533
6 XUL mozilla::EventStateManager::HandleCrossProcessEvent dom/events/EventStateManager.cpp:1247
7 XUL PLDHashTable::Remove xpcom/ds/PLDHashTable.cpp:637
8 XUL mozilla::EventStateManager::PostHandleEvent dom/events/EventStateManager.cpp:2951
9 XUL mozilla::EventDispatcher::Dispatch xpcom/base/nsCOMPtr.h:328

Something is certainly going wrong with that .str.1.llvm.2376781057090901720 in the signature, but it sounds like there is a specific drag and drop issue, too. Off hand, I'd guess it could be some kind of OOM issue.

It looks like these all have the IPC error "unknown union type".

Crash Signature: [@ mozilla::ipc::FatalError | .str.1.llvm.2376781057090901720] → [@ mozilla::ipc::FatalError | .str.1.llvm.2376781057090901720] [@ mozilla::ipc::FatalError | mozilla::ipc::IProtocol::HandleFatalError | mozilla::ipc::IPDLParamTraits<T>::Write ]
Summary: Crash in [@ mozilla::ipc::FatalError | .str.1.llvm.2376781057090901720] → Crash with "unknown union type" in PContentParent::SendInvokeDragSession

I found what is probably the older signature before LLVM weirdness by looking for crashes where the protosignature contains PContentParent::SendInvokeDragSession.

I took a quick look at the code, and maybe I'm missing something but it looks like some of these error cases can create an IPCDataTransferItem but not assign to its data member, leaving it in the invalid default-constructed state which is caught by the assertion.

(Edited to clarify that I didn't mean uninitialized in the dangerous C sense.)

Component: IPC → DOM

evilpie, could this be a regression from bug 1497831.

Flags: needinfo?(evilpies)
Priority: -- → P2

Quite possible, this function is extremely convoluted. Some of the faulty error handling was there before as well.

Flags: needinfo?(evilpies)
Flags: needinfo?(evilpies)

I think the crash signatures are too broad, because there various other IPC messages like printing etc.

That's 99 of the 130 total crashes with this signature in the last week.

I don't know how to reproduce these crashes, but I concur with comment 2 and comment 4, that adding error handling should fix those "unknown union errors".

ConvertToShmem should really do error handling, i.e return Maybe<Shmem>? I am not sure how much of if (!dataAsShmem.IsReadable() || !dataAsShmem.Size<char>()) { is just cargo-culted, especially the IsReadable() part.

Assignee: nobody → evilpies
Flags: needinfo?(evilpies)
Pushed by evilpies@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/fe805cb52ee2
Fix error handling in TransferableToIPCTransferable. r=smaug
Status: NEW → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67

Please nominate this for Beta approval if you're comfortable doing so.

Flags: needinfo?(evilpies)

Comment on attachment 9047938 [details]
Bug 1530519 - Fix error handling in TransferableToIPCTransferable. r?smaug

Beta/Release Uplift Approval Request

  • Feature/Bug causing the regression: None
  • 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?: No
  • If yes, steps to reproduce: We can't reproduce this
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This is a pretty obvious improvement with a small code change
  • String changes made/needed:
Flags: needinfo?(evilpies)
Attachment #9047938 - Flags: approval-mozilla-beta?

Comment on attachment 9047938 [details]
Bug 1530519 - Fix error handling in TransferableToIPCTransferable. r?smaug

Low risk crash fix, let's take it for beta 14.

Attachment #9047938 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.