New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

For the CallbackGuard, print something to stderr and use abort() inst… #225

Merged
merged 1 commit into from Aug 30, 2017

Conversation

Projects
None yet
3 participants
@sdroege
Member

sdroege commented Aug 30, 2017

…ead of exit()

Otherwise the process just silently exits without anybody knowing what's
wrong. With abort() we also make sure that no at_exit handlers are
called and it's easier to attach a debugger and/or get a coredump.

Fixes #224

For the CallbackGuard, print something to stderr and use abort() inst…
…ead of exit()

Otherwise the process just silently exits without anybody knowing what's
wrong. With abort() we also make sure that no at_exit handlers are
called and it's easier to attach a debugger and/or get a coredump.

Fixes #224
if thread::panicking() {
process::exit(101);
let _ = stderr().write(b"Uncaught panic, exiting");

This comment has been minimized.

@sdroege

sdroege Aug 30, 2017

Member

Anything more useful to print here, anything to add?

@sdroege

sdroege Aug 30, 2017

Member

Anything more useful to print here, anything to add?

This comment has been minimized.

@EPashkin

EPashkin Aug 30, 2017

Member

IMHO nothing.

@EPashkin

EPashkin Aug 30, 2017

Member

IMHO nothing.

This comment has been minimized.

@sdroege

sdroege Aug 30, 2017

Member

Actually something. It needs a \n at the end. @GuillaumeGomez , can you just push that simple change? Going through a PR seems a bit too much overhead for one character :)

@sdroege

sdroege Aug 30, 2017

Member

Actually something. It needs a \n at the end. @GuillaumeGomez , can you just push that simple change? Going through a PR seems a bit too much overhead for one character :)

This comment has been minimized.

@GuillaumeGomez

GuillaumeGomez Aug 30, 2017

Member

Ah sure.

@GuillaumeGomez

GuillaumeGomez Aug 30, 2017

Member

Ah sure.

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Aug 30, 2017

Member

👍

Member

EPashkin commented Aug 30, 2017

👍

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Aug 30, 2017

Member

Thanks!

Member

GuillaumeGomez commented Aug 30, 2017

Thanks!

@GuillaumeGomez GuillaumeGomez merged commit a9ccf5b into gtk-rs:master Aug 30, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@EPashkin EPashkin referenced this pull request Aug 30, 2017

Merged

New regen #15

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment