450531 - Mac assertions shouldn't be fatal

Status: RESOLVED DUPLICATE of bug 668469

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

bug 417564 introduced marcros to catch exception. If I get right then if exception cannot be processed then firefox is crashed:

// We need to raise a mach-o signal here, the Mozilla crash reporter on
// Mac OS X does not respond to POSIX signals. Raising mach-o signals directly
// is tricky so we do it by just derefing a null pointer.

But it seems every assertion like NSAssert1 rise an exception NSInternalInconsistencyException:

The NSAssert1 macro evaluates the condition and serves as a front end to the assertion handler. 
Each thread has its own assertion handler, which is an object of class NSAssertionHandler. When 
invoked, an assertion handler prints an error message that includes the method and class names (or 
the function name). It then raises an exception of type NSInternalInconsistencyException.

Therefore often my firefox build is crashed, for example I get crash in accessible/src/mac/mozAccessible.mm:582:

- (void)didReceiveFocus
{
  NS_OBJC_BEGIN_TRY_ABORT_BLOCK;

#ifdef DEBUG_hakan
  NSLog (@"%@ received focus!", self);
#endif
  NSAssert1(![self accessibilityIsIgnored], @"trying to set focus to ignored element! (%@)", self);
  NSAccessibilityPostNotification(GetObjectOrRepresentedView(self),
                                  NSAccessibilityFocusedUIElementChangedNotification);

  NS_OBJC_END_TRY_ABORT_BLOCK;
}

I'm not sure what is the bug here. Possibly these assertions should be ignored if they cannot be handled or I don't know. But I'm pretty sure they shouldn't lead to crash.

Comment 1

[Steven Michaud [:smichaud] (Retired)]

I think it's better to rephrase the question as follows -- "Should
some uses of NSAssert be replaced by non-fatal alternatives?"

Aside from one reference in the Camino source, all usage of NSAssert()
(and friends) in the Mozilla tree is in mozAccessible.mm and
mozDocAccessible.mm (in accessible/src/mac/), where it was added a
long time ago (2006), when calls to NSAssert() didn't (yet) trigger a
crash.

So we need (I think) to go over that code and see if we really want
the browser to crash on the errors where NSAssert() is currently used.

Hwaara:  Since you're the one who added all the calls to NSAssert(),
I'm cc-ing you here :-)

Comment 2

[Steven Michaud [:smichaud] (Retired)]

Side note:  As far as I know, calls to NSAssert() are (now) fatal in both opt and debug builds.

Comment 3

[Håkan Waara]

I just used it as a more more objc-y version of NS_ASSERTION since (IIRC) it handles %@ arguments right, etc.  

I'm sure I never expected a crash if the assertion failed. :-)

Comment 4

[alexander :surkov (:asurkov)]

I believe assertion is good thing to bring developer attention there is a problem in the code. If we get an assertion in the accessibility method then it means some stuffs on the web page are inaccessible and we shouldn't crash firefox what means firefox is inaccessible for the certain webpage entirely. Let's imagine I debug some problem and get the crash on assertion in the place I didn't expect and that means I should switch to new problem. It's awful thing. So for example I like more windows assertions where I get alert window if assertion is occurred. I would like to have something similar on Mac.

Comment 5

[Steven Michaud [:smichaud] (Retired)]

By the way, I've just confirmed that calling NSAssert() in an opt
build does trigger an NSInternalInconsistencyException (and a crash).

Comment 6

[Daniel Einspanjer [:dre] [:deinspanjer]]

I just commented in the related bug 470864 comment 37 that this has been hitting me a lot (especially bad today) and I linked several of my crash reports.

Comment 7

[David Bolter [:davidb] (NeedInfo me for attention)]

Hubert, feel like triaging this bug?

Comment 8

[Hubert Figuiere [:hub]]

We can override assertion handlers on Mac and therefor provide whatever we feel is useful.

Comment 9

[Hubert Figuiere [:hub]]

That assert is bug 668469.