Skip to content
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

Hexyl don't respond to ctrl-c #84

Closed
secworks opened this issue Feb 11, 2020 · 12 comments
Closed

Hexyl don't respond to ctrl-c #84

secworks opened this issue Feb 11, 2020 · 12 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@secworks
Copy link

On MacOS, when running hexyl without any input the application can't be terminated with ctrl-c. Like this:

Screenshot 2020-02-11 at 16 53 29

This is with hexyl 0.6.0 on MacOS 10.14.6

@sharkdp sharkdp added the bug Something isn't working label Feb 11, 2020
@twe4ked
Copy link

twe4ked commented Feb 13, 2020

Looks like this is a regression.

$ hexyl
┌────────┬─────────────────────────┬─────────────────────────┬────────┬────────┐
^C
$ hexyl --version
hexyl 0.3.0
$ brew upgrade hexyl
==> Upgrading 1 outdated package:
hexyl 0.3.0 -> 0.6.0
==> Upgrading hexyl
==> Downloading https://homebrew.bintray.com/bottles/hexyl-0.6.0.mojave.bottle.1.tar.gz
==> Downloading from https://akamai.bintray.com/99/99a3133f73538f29f0dc2a603f122b2c2165fdd53de0f2141fc0ebab086c22f2?__gda__=exp=1581631705~hmac=3d6116586601720ea23e57ef75b254d0f67b4fe494e7be2d3aeb175112ad9eb7&response-content-disposition=atta
######################################################################## 100.0%
==> Pouring hexyl-0.6.0.mojave.bottle.1.tar.gz
🍺  /usr/local/Cellar/hexyl/0.6.0: 5 files, 704.3KB
Removing: /usr/local/Cellar/hexyl/0.3.0... (5 files, 1.2MB)
==> Checking for dependents of upgraded formulae...
==> No dependents found!
$ hexyl
^C^C^C^C

@sharkdp sharkdp added the help wanted Extra attention is needed label Feb 16, 2020
@Qyriad
Copy link
Contributor

Qyriad commented Feb 17, 2020

This affects me on Linux as well. Kernel 5.5.3 (Arch Linux), Hexyl 0.6.0.

@twe4ked
Copy link

twe4ked commented Feb 17, 2020

Looks like it was broken here 1903047. The handler gets called but the cancelled never gets loaded if read doesn't return (AFAICT).

@secworks
Copy link
Author

Not very good at reading Rust, but as far as I understand the logic flow that seems like a correct analysis.

@sharkdp
Copy link
Owner

sharkdp commented Feb 18, 2020

@twe4ked Yes, that is correct. The handler doesn't work because we are in the blocking read call. This can easily be verified by pressing Ctrl-C and then Enter.

Simply reverting that commit would be an easy fix for this issue, but make the #35 situation worse. However, this bug here is much more relevant. #35 is a "cosmetic" thing only.

@bjorn3
Copy link

bjorn3 commented Feb 18, 2020

libc::read will return EINTR when a signal handler was called. The read method for stdio and files in libstd loops until it doesn't return EINTR anymore. Try using the raw libc::read function.

@sharkdp
Copy link
Owner

sharkdp commented Feb 18, 2020

@bjorn3: Thank you!
It looks like setting SA_RESTART in sigaction(3) would also work?

I'm currently looking into the signal_hook crate...

@sharkdp
Copy link
Owner

sharkdp commented Feb 18, 2020

No, I guess that wouldn't work. Unsetting SA_RESTART would work if it wouldn't be caught explicitly.

@secworks
Copy link
Author

secworks commented Mar 9, 2020

Any new status on this issue? I can perform testing on MacOS if that is needed.

@sharkdp
Copy link
Owner

sharkdp commented Mar 9, 2020

I did not find any solution with signal_hook and I would like to refrain from using unsafe features (callibg libc::read directly).

I'm going to remove the signal handler completely, for now. This will fix this issue.

@sharkdp
Copy link
Owner

sharkdp commented Mar 9, 2020

fixed in v0.7.0

@secworks
Copy link
Author

Cool!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

5 participants