Closed Bug 1471504 Opened Last year Closed Last year

Crash in [@ CrashReporter::TerminateHandler | std::__terminate][@ mozAccessible parent]

Categories

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

62 Branch
Unspecified
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox61 --- unaffected
firefox62 + fixed
firefox63 --- fixed

People

(Reporter: whimboo, Assigned: spohl)

References

Details

(Keywords: crash, regression)

Crash Data

This bug was filed from the Socorro interface and is
report bp-acfd9ef3-fc93-4f3d-9565-86e740180626.
=============================================================

Top 10 frames of crashing thread:

0 XUL CrashReporter::TerminateHandler toolkit/crashreporter/nsExceptionHandler.cpp:1435
1 libc++abi.dylib std::__terminate 
2 libc++abi.dylib __cxa_throw 
3 libobjc.A.dylib objc_exception_throw 
4 CoreFoundation +[NSException raise:format:arguments:] 
5 Foundation -[NSAssertionHandler handleFailureInMethod:object:file:lineNumber:description:] 
6 XUL -[mozAccessible parent] accessible/mac/mozAccessible.mm:595
7 XUL -[mozAccessible accessibilityAttributeValue:] accessible/mac/mozAccessible.mm:286
8 AppKit NSAccessibilityGetObjectForAttributeUsingLegacyAPI 
9 AppKit ___NSAccessibilityEntryPointValueForAttribute_block_invoke.804 

=============================================================
Flags: needinfo?(jwatt)
this bucket of crashes here makes up the bulk of the remaining [@ CrashReporter::TerminateHandler | std::__terminate ] signature on 62.0b6 after bug 1467582 got fixed - overall that's accounting for 20% of mac browser crashes in beta 6.
it seems to be mostly ru/uk builds that are affected by this type of crash.
one comment says it's repeatedly happening after hitting the ESC key to leave from fullscreen mode while playing a video.
OK, that shows about 100 crashes in the last week, on beta 62. 
I think jwatt is just back from PTO so he will likely investigate soon.
By way of background, the patch for bug 1270217 was created because Emilio's patch for bug 1465585 to switch us from using mozilla::Move to std::move broke local builds (not CI builds) on Mac since the headers for 10.7 don't have std::move. I wrote the patch to update us to 10.9, but that was just because I was one of the first people to bump into the local bustage rather than anything else.

I don't actually have much experience of using/debugging macOS platform libs so I'll probably be quite slow to debug this. I'll can certainly work on it, but maybe dmajor would be willing to take a look since I'm sure he'd manage to figure things out a lot faster.
Flags: needinfo?(jwatt) → needinfo?(dmajor)
Oops, it was spohl that fixed bug 1467582, not dmajor. I also didn't mean to remove the needinfo for myself.
Flags: needinfo?(spohl.mozilla.bugs)
Flags: needinfo?(jwatt)
Flags: needinfo?(dmajor)
Adding another crash signature I spotted which has mozAccessible parent in the stack, as well as CrashReporter::TerminateHandler().
Crash Signature: [@ CrashReporter::TerminateHandler | std::__terminate] → [@ CrashReporter::TerminateHandler | std::__terminate] [@ __cficu_udat_toPatternRelativeDate]
This is an intentional crash in our accessibility code, seemingly due to the inability of finding a parent when we expect one:

https://hg.mozilla.org/releases/mozilla-beta/annotate/1716b76aa7b8df189e74bf87394d3a11e0c5ac12/accessible/mac/mozAccessible.mm#l595

Joanmarie, I see that you have been active in this file recently. Do you have any thoughts here?
Flags: needinfo?(spohl.mozilla.bugs) → needinfo?(jdiggs)
NI Jamie for awareness. This is currently tracking 62, but might be fixed? (Not many recent crashes?)
Flags: needinfo?(jteh)
(In reply to Stephen A Pohl [:spohl] from comment #8)
> This is an intentional crash in our accessibility code, seemingly due to the
> inability of finding a parent when we expect one:

True enough, but why would bug 1270217 cause this to manifest? Also (I'm not a Mac developer), is NSAssert1 really intended to run on release builds? Was this assertion perhaps previously captured by our native exception handling? And if so, would that be also fixed by bug 1290972? or are you suggesting that the correlation with bug 1270217 is bogus?

Unfortunately, we don't have anyone on the a11y team who really knows anything about Mac native code. However, I did a bit of poking around. It seems that ProxyAccessible::OuterDocOfRemoteBrowser (as called at mozAccessible.mm line 588) can return null; see bug 1171728 comment 6. If that happens, we'd certainly hit that assertion. (It *shouldn't* happen, but we've seen it in the wild, as above.)

So, the fix here (for now) is to convert that NSAssert1 into some kind of assert that only fires on debug builds (we still want to be able to debug this if a developer sees it) and return null in that case:

  SomeKindOfDebugOnlyNSAssert1(nativeParent, @"!!! we can't find a parent for %@", self);
  if (!nativeParent) {
    return nil;
  }

  return GetObjectOrRepresentedView(nativeParent);
Flags: needinfo?(spohl.mozilla.bugs)
Flags: needinfo?(jteh)
Flags: needinfo?(jdiggs)
Might also be good to add a debug only assert for outerDoc earlier.
Bug 1290972 landed, which appears to fix this.
Blocks: 1290972
No longer blocks: 1270217
Flags: needinfo?(spohl.mozilla.bugs)
Flags: needinfo?(jwatt)
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Crash Signature: [@ CrashReporter::TerminateHandler | std::__terminate] [@ __cficu_udat_toPatternRelativeDate] → [@ CrashReporter::TerminateHandler | std::__terminate] [@ __cficu_udat_toPatternRelativeDate] [@ objc_exception_throw | +[NSException raise:format:arguments:] | -[NSAssertionHandler handleFailureInMethod:object:file:lineNumber:description:]]
Crash Signature: [@ CrashReporter::TerminateHandler | std::__terminate] [@ __cficu_udat_toPatternRelativeDate] [@ objc_exception_throw | +[NSException raise:format:arguments:] | -[NSAssertionHandler handleFailureInMethod:object:file:lineNumber:description:]] → [@ CrashReporter::TerminateHandler | std::__terminate] [@ __cficu_udat_toPatternRelativeDate] [@ _objc_exception_do_catch] [@ libobjc.A.dylib@0x14da4] [@ objc_exception_throw | +[NSException raise:format:arguments:] | -[NSAssertionHandler handleFailur…
Assignee: nobody → spohl.mozilla.bugs
Target Milestone: --- → mozilla63
Bug 1290972 has been uplifted to 62.
You need to log in before you can comment on or make changes to this bug.