1631276 - Crash in [@ mozilla::ipc::FatalError | mozilla::ipc::IProtocol::HandleFatalError | mozilla::a11y::PDocAccessibleParent::SendChildAtPoint]

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect, P1)

Description

[[:philipp]]

This bug is for crash report bp-44369265-17da-4a1c-896a-7a8aa0200419.

Top 10 frames of crashing thread:

0 XUL mozilla::ipc::FatalError ipc/glue/ProtocolUtils.cpp:160
1 XUL mozilla::ipc::IProtocol::HandleFatalError const ipc/glue/ProtocolUtils.cpp:399
2 XUL mozilla::a11y::PDocAccessibleParent::SendChildAtPoint ipc/ipdl/PDocAccessibleParent.cpp
3 XUL mozilla::a11y::ProxyAccessible::ChildAtPoint accessible/ipc/other/ProxyAccessible.cpp:796
4 XUL -[mozAccessible accessibilityHitTest:] accessible/mac/mozAccessible.mm:526
5 XUL -[mozAccessible accessibilityHitTest:] accessible/mac/mozAccessible.mm:519
6 AppKit -[NSWindow accessibilityHitTest:] 
7 AppKit -[NSApplication accessibilityHitTest:] 
8 AppKit CopyElementAtPosition 
9 HIServices _AXXMIGCopyElementAtPosition 

this crash signature from macos users is starting to show up since firefox 76 - perhaps related to bug 1598299. it's only affecting nightly and devedition users though...

Comment 1

[Tim Spurway [:tspurway]]

:jamie, if it's a P1 can you get someone to work on it?

Comment 2

[James Teh [:Jamie]]

I've been trying to figure out what's going on here, but I'm honestly at a bit of a loss. I'll keep trying. Crash volume is pretty low - 15 crashes so far - but I'll leave it as p1 for now.

Comment 3

[James Teh [:Jamie]]

This is still low volume. I'm wondering whether this will just go away with the Mac AccessibleOrproxy refactor (bug 1632252 ). I'm inclined to wait for that and see what happens.

Comment 4

[James Teh [:Jamie]]

This is still super low volume, but I see we have some crashes on Linux such as this one:
bp-c1626022-2d36-46c9-88b4-6b6450200610
as well as crashes on Mac after bug 1632252 such as this one:
bp-77d03964-76f2-4a56-b8a7-ef5e00200610
So this is still a problem and not Mac specific.

The crash is caused by the IPC error "Error deserializing 'PDocAccessible'". I assume that's the aResultDoc return for PDocAccessible::ChildAtPoint. So, somehow, the result document actor is bad.

The result doc is fetched by calling Document()->IPCDoc() on the hit test target accessible. I don't understand how this could be invalid. If it is dying, IPCDoc() should have returned null.

Comment 5

[James Teh [:Jamie]]

Nika, do you know if the "Error deserializing 'PDocAccessible'" fatal IPC error could be caused by the following scenario?

1. The DocAccessibleChild gets created in the content process.
2. We send the constructor to the parent process, which I assume is an async message. So, as far as the content process is concerned, the actor is all set up. (Or is there some construction handshake between content and parent here?)
3. The parent process doesn't process the queue yet. Instead, it answers an a11y query, which makes a sync IPC call.
4. The sync IPC call sends back the actor created in (1) as a return value.
5. Because (3) is sync IPC, the parent process tries to deserialise (4), even though it hasn't yet processed the async constructor sent in (2).

If this is plausible, is there some way I can detect this?

Comment 6

[James Teh [:Jamie]]

I guess I could create my own constructor response message, send it from parent once construction is done, set a flag on the DocAccessibleChild when it's received and only allow this sync message to send a DocAccessibleChild as a return value if that flag is set. That feels really yucky, though, and creates more IPC traffic which won't even be useful 99% of the time. I'm really hoping there's another way...

Comment 7

[Nika Layzell [:nika] (ni? for response)]

(In reply to James Teh [:Jamie] from comment #5)

Nika, do you know if the "Error deserializing 'PDocAccessible'" fatal IPC error could be caused by the following scenario?

Yes, that's definitely possible to have happen. It's a big issue with sync IPC where ordering guarantees are completely broken as soon as there are nested sync IPC messages, and it's one of the reasons why we're trying to get rid of nested sync IPC, and it's (mostly) only used in a11y and NPAPI nowadays.

I can think of 3 ways to do this:

1. The ack message you mentioned in comment 6
2. Sending an ID rather than the actor in the message, and looking it up manually in the parent process through some other mechanism, or
3. Using epochs in the ChildAtPoint message and the PDocAccesible constructor to keep track of what the most recent PDocAccessible the parent process has seen so far is.

The easiest option is probably the ack message.

Comment 8

[James Teh [:Jamie]]

Attachment: Bug 1631276 part 1: Move Set/GetIsConstructedInParentProcess into DocAccessibleChildBase.

Comment 9

[James Teh [:Jamie]]

Attachment: Bug 1631276 part 2: On non-Windows, have the parent process notify the content process when the DocAccessibleParent is constructed.

This is already handled in a different way on Windows.

Comment 10

[James Teh [:Jamie]]

Attachment: Bug 1631276 part 3: Don't return a descendant document in DocAccessibleChild::RecvChildAtPoint unless we're certain that document has been constructed in the parent process.

Comment 11

[James Teh [:Jamie]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ccd2df818f52fd4b60d9b61b9615443271b46838

Comment 12

[James Teh [:Jamie]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0872e8147b5415a1d9ea3bd0b521fe3d8c7a5265

Comment 13

[James Teh [:Jamie]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=712459e836cbf4c034074f9f59b24041d6567243

Comment 14

[James Teh [:Jamie]]

Seriously, I'll get this right eventually...
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e865415ebe35fd37f0cecd206af1343c2fdae55d

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/26a4a3ed96f5
part 1: Move Set/GetIsConstructedInParentProcess into DocAccessibleChildBase. r=eeejay
https://hg.mozilla.org/integration/autoland/rev/33231d9cbc84
part 2: On non-Windows, have the parent process notify the content process when the DocAccessibleParent is constructed. r=eeejay
https://hg.mozilla.org/integration/autoland/rev/f75850ba8c68
part 3: Don't return a descendant document in DocAccessibleChild::RecvChildAtPoint unless we're certain that document has been constructed in the parent process. r=eeejay

Comment 16

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/26a4a3ed96f5
https://hg.mozilla.org/mozilla-central/rev/33231d9cbc84
https://hg.mozilla.org/mozilla-central/rev/f75850ba8c68