Mir

Code review comment for lp:~vanvugt/mir/fatal-error

Revision history for this message
Chris Halse Rogers (raof) wrote :

Hm.

So, I think we're actually talking about two different things here: crashes and “crashes”, which are really uncaught exceptions.

You obviously can't know what your crashes will be in advance. On the other hand, you know *exactly* what your “crashes” will be, because you deliberately put them in there.

A core file is good (or at least usually sufficient) for crashes, because the cause of the crash is the thing the program is trying to do _right now_, so it's highly likely that the state of interest is the current state.

For uncaught exceptions the current state is frequently not the interesting state - when unity-system-compositor fails to start with “could not find drm device”, the interesting data is in previous iterations of the “probe this udev device” loop. It's idiomatic to try and do a bunch of things, and if they *all* fail, throw.

Clean core files are useful, and mir::abort() looks to be useful, so I'm not disapproving.

However, most of the places you're calling mir::abort() in this branch are potentially temporary failures, and it _is_ inappropriate to abort there - failure to schedule pageflips, failure to set one crtc (out of potentially more than one active crtc), and failure in eglSwapBuffers are all potentially transitory or recoverable problems which this branch turns into a SIGABRT.

If we're raising these exceptions on a non-main thread we should stop doing that (or catch them and propagate them _to_ the main thread), though.

review: Needs Fixing

« Back to merge proposal