support RELEASE_ASSERT and DEBUG_ASSERT for release and debug mode

[RJ Skerry-Ryan]

From the discussion in https://github.com/mixxxdj/mixxx/pull/390:

Add a RELEASE_ASSERT macro:
* fatal (quits immediately) always
* prints a message to the log about the failure

Add a DEBUG_ASSERT macro:
* compiled away in release builds
* in debug builds, print a failure warning
* if DEBUG_ASSERT_FATAL defined, quit immediately

Add a scons flag, debug_asserts_fatal, default to 0.
* Define DEBUG_ASSERT_FATAL

[Sean M. Pappalardo (pegasus-renegadetech)]

I wasn't following the discussion in the PR so I'll chime in here:
- We never want to quit immediately in a release build! We want to recover as much as possible.
- Doesn't ErrorHandler already address the RELEASE_ASSERT case, in that you can call it with a critical error and it will log it and attempt a graceful shutdown? (It just needs to be macro-ized.)
- Qt docs say that Q_ASSERTs are intended for development and are meant to be disabled in release builds. We should really add error-handling code after all asserts as if they aren't there so we can recover from problems as gracefully as possible for each one.

[Sean M. Pappalardo (pegasus-renegadetech)]

Also, why worry about people using development builds from the build server live? Why can't we just produce nightly release builds too for those people?

[Owen Williams (ywwg)]

I think the idea behind RELEASE_ASSERT is that it is only used if the program is about to crash anyway (in an unrecoverable fashion), so we might as well say why.

[RJ Skerry-Ryan (rryan)]

Sean -- please read the PR.

RELEASE_ASSERT would be extremely restricted in use (similar to our current use of Q_ASSERT) where it's only for cases where it's absolutely impossible to gracefully recover.

Rest assured, I will not let gratuitous RELEASE_ASSERTS in :).

[RJ Skerry-Ryan (rryan)]

> Also, why worry about people using development builds from the build server live? Why can't we just produce nightly release builds too for those people?

Some distros build Mixxx in debug mode. Some users follow our compilation instructions without thinking through the implications. I'm strongly for opt-in so people are aware of the risk.

[RJ Skerry-Ryan (rryan)]

> Qt docs say that Q_ASSERTs are intended for development and are meant to be disabled in release builds. We should really add error-handling code after all asserts as if they aren't there so we can recover from problems as gracefully as possible for each one.

RELEASE_ASSERT is only for the situation where you really can't recover. We should work to remove those situations but the fact is that they exist currently and we just Q_ASSERT currently.

[Sean M. Pappalardo (pegasus-renegadetech)]

(Of course I read the PR before commenting here.)

This just sounds like reinventing the wheel. How is adding these macros any different from having proper error handling code and disabling asserts in release builds?

As for distros that package debug builds and blind-compilation users, we should change our default build flags to be release ones.

[RJ Skerry-Ryan (rryan)]

Fixed in:

https://github.com/mixxxdj/mixxx/commit/1eaca501c86ea7004f382107c485e2014cfadae3
and
https://github.com/mixxxdj/mixxx/commit/d15a2b97cbabd689a1b43c17c2328749b117fbc9

[Sean M. Pappalardo (pegasus-renegadetech)]

Okaaaay... this just feels like it's going to come back to bite us. It's additional complexity at the very least.

[RJ Skerry-Ryan (rryan)]

You're fencing with a straw man.

I got rid of 54 Q_ASSERTs. Not a single one of them had graceful error handling. If we compiled with Q_ASSERTs off Mixxx would segfault right after. Now every single one of them has graceful error handling and will not crash a release build.

[Swiftb0y (swiftb0y)]

Mixxx now uses GitHub for bug tracking. This bug has been migrated to:
https://github.com/mixxxdj/mixxx/issues/7661