search

sharkdp / hexyl

A command-line hex viewer
Apache License 2.0
8.92k stars 227 forks source link

Hexyl don't respond to ctrl-c #84

Closed secworks closed 4 years ago

secworks commented 4 years ago

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

twe4ked commented 4 years ago

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
Qyriad commented 4 years ago

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

twe4ked commented 4 years ago

Looks like it was broken here https://github.com/sharkdp/hexyl/commit/1903047. The handler gets called but the cancelled never gets loaded if read doesn't return (AFAICT).

secworks commented 4 years ago

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

sharkdp commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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

secworks commented 4 years ago

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

sharkdp commented 4 years ago

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 commented 4 years ago

fixed in v0.7.0

secworks commented 4 years ago

Cool!