Closed Bug 1455745 Opened Last year Closed Last year

Crash in mozilla::a11y::CreateHolderFromAccessible

Categories

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

Unspecified
Windows 10
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- disabled
firefox59 --- unaffected
firefox60 --- disabled
firefox61 --- fixed

People

(Reporter: mccr8, Assigned: Jamie)

References

Details

(Keywords: crash)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is
report bp-c195137e-f7db-4a10-8924-150880180419.
=============================================================

Top 10 frames of crashing thread:

0 xul.dll mozilla::a11y::CreateHolderFromAccessible accessible/ipc/win/COMPtrTypes.cpp:54
1 xul.dll mozilla::a11y::DocAccessible::DoInitialUpdate accessible/generic/DocAccessible.cpp:1474
2 xul.dll mozilla::a11y::DocAccessibleWrap::DoInitialUpdate accessible/windows/msaa/DocAccessibleWrap.cpp:146
3 xul.dll mozilla::a11y::NotificationController::WillRefresh accessible/base/NotificationController.cpp:666
4 xul.dll nsRefreshDriver::Tick layout/base/nsRefreshDriver.cpp:1882
5 xul.dll mozilla::RefreshDriverTimer::TickDriver layout/base/nsRefreshDriver.cpp:337
6 xul.dll mozilla::InactiveRefreshDriverTimer::TickOne layout/base/nsRefreshDriver.cpp:958
7 xul.dll nsTimerImpl::Fire xpcom/threads/nsTimerImpl.cpp:701
8 xul.dll nsTimerEvent::Run xpcom/threads/TimerThread.cpp:290
9 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1096

=============================================================

#3 Windows top crash for the 4-17 Nightlies, though that is mostly from a single user. These crashes are all
  MOZ_RELEASE_ASSERT((((HRESULT)(hr)) >= 0))

The last comment in bug 1399557 is "No, we've since landed some patches to get rid of those crashes on Nightly." but it looks like this is still happening almost two months later.
See Also: → 1399557
Jamie, do you think this is another location that we should convert to MOZ_ASSERT?
Flags: needinfo?(jteh)
I think so. While I'd obviously prefer to fix this properly, these crashes aren't helping us achieve this at this stage.

I think we'd also need to convert the assert in accessible/generic/DocAccessible.cpp:1475.
Assignee: nobody → jteh
Flags: needinfo?(jteh)
Looking at the crash dump from comment 0, the HRESULT is 0x80040153 "Invalid value for registry". That's... intriguing. Any ideas at all, Aaron? The obvious answer is that something COM or IAccessible related is pretty badly messed up in the user's registry, but I'm not sure if we have much chance of figuring out exactly what. Our activation context should supply most of what we need.

I'll still go ahead and submit a patch to convert these to asserts.
Flags: needinfo?(aklotz)
Comment on attachment 8970067 [details]
Bug 1455745: Remove more accessibility related diagnostic crashes on Nightly.

https://reviewboard.mozilla.org/r/238820/#review244620

::: accessible/generic/DocAccessible.cpp:1481
(Diff revision 1)
>          int32_t childID = AccessibleWrap::GetChildIDFor(this);
>  #else
>          int32_t holder = 0, childID = 0;
>  #endif
>          tabChild->SendPDocAccessibleConstructor(ipcDoc, nullptr, 0, childID,
>                                                  holder);

should you actually proceed with it having IsNull() holder?
Comment on attachment 8970067 [details]
Bug 1455745: Remove more accessibility related diagnostic crashes on Nightly.

https://reviewboard.mozilla.org/r/238820/#review244620

> should you actually proceed with it having IsNull() holder?

This will certainly cause broken a11y (failing calls) from the client's perspective, but our code handles this without crashing:
1. It will trigger an assert in the parent process, but that's already a MOZ_ASSERT, not a MOZ_DIAGNOSTIC_ASSERT. See https://searchfox.org/mozilla-central/source/dom/ipc/TabParent.cpp#972
2. See protection in https://searchfox.org/mozilla-central/source/accessible/ipc/win/ProxyAccessible.h#48
(In reply to James Teh [:Jamie] from comment #6)

> > should you actually proceed with it having IsNull() holder?
> 
> This will certainly cause broken a11y (failing calls) from the client's
> perspective, but our code handles this without crashing:
> 1. It will trigger an assert in the parent process, but that's already a
> MOZ_ASSERT, not a MOZ_DIAGNOSTIC_ASSERT. See
> https://searchfox.org/mozilla-central/source/dom/ipc/TabParent.cpp#972
> 2. See protection in
> https://searchfox.org/mozilla-central/source/accessible/ipc/win/
> ProxyAccessible.h#48

ok, I see, thank you for the pointers
Comment on attachment 8970067 [details]
Bug 1455745: Remove more accessibility related diagnostic crashes on Nightly.

https://reviewboard.mozilla.org/r/238820/#review246544
Attachment #8970067 - Flags: review?(aklotz) → review+
(In reply to James Teh [:Jamie] from comment #3)
> Looking at the crash dump from comment 0, the HRESULT is 0x80040153 "Invalid
> value for registry". That's... intriguing. Any ideas at all, Aaron? The
> obvious answer is that something COM or IAccessible related is pretty badly
> messed up in the user's registry, but I'm not sure if we have much chance of
> figuring out exactly what. Our activation context should supply most of what
> we need.
> 
> I'll still go ahead and submit a patch to convert these to asserts.

Yeah, some stuff must be pretty messed up. We do have logging in place in certain areas to annotate crash reports with the contents of the registry, but we don't have those in this case.
Flags: needinfo?(aklotz)
hg error in cmd: hg pull upstream: pulling from https://hg.mozilla.org/integration/autoland
searching for changes
abort: HTTP Error 500: Internal Server Error
Pushed by jteh@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c36098a8e83f
Remove more accessibility related diagnostic crashes on Nightly. r=aklotz
https://hg.mozilla.org/mozilla-central/rev/c36098a8e83f
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.