Page MenuHomeWildfire Games

Do not actually Kill in debug_break() in non-debug binaries
ClosedPublic

Authored by wraitii on May 19 2018, 7:25 AM.

Details

Reviewers
None
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP21998: Do not actually Kill in debug_break() in non-debug binaries
Summary

Revelead by the A23 crash when hosting a game in the lobby. We have a debug_break() call that calls "kill" with SIGTRAP, which generally doesn't get handled correctly in release versions, particularly on OSX.

I believe this should just do nothing in release builds.

Test Plan

Agree.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
_invalid_texture_fix
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 6084
Build 10131: Vulcan BuildJenkins
Build 10130: arc lint + arc unit

Event Timeline

wraitii created this revision.May 19 2018, 7:25 AM
Vulcan added a subscriber: Vulcan.May 19 2018, 7:29 AM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/511/display/redirect

This revision was not accepted when it landed; it landed in state Needs Review.Jan 1 2019, 5:07 PM
This revision was automatically updated to reflect the committed changes.
elexis added a subscriber: elexis.Jan 1 2019, 5:37 PM

Who needs arguments when we can rely on belief

Stan added a subscriber: Stan.Jan 1 2019, 5:38 PM

'O ye, of little faith'

Well you just had to ask. Also "I believe" above should be read as "In my opinion".

The reasoning here is that debug_break() is intended to provide a break point on debug releases where you can jump in debug. It ends up killing the app in release versions.
Given where this function is called (some texture loading and other recoverable errors), it makes no sense to kill the app in release mode.

This whole family of functions should imo be removed anyways, it's dead and useless code for the most part.