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

Handle interrupted error from mio #281

Merged
merged 2 commits into from
Feb 20, 2021
Merged

Handle interrupted error from mio #281

merged 2 commits into from
Feb 20, 2021

Conversation

rschoon
Copy link
Contributor

@rschoon rschoon commented Feb 14, 2021

mio 0.7 was changed to no longer automatically retry on system call interruption, so we now must do it ourselves. For example, the process receiving SIGINT could cause notify to panic, if the signal handler had been changed from default:

thread '<unnamed>' panicked at 'poll failed: Os { code: 4, kind: Interrupted, message: "Interrupted system call" }', /home/rschoon/.cargo/registry/src/github.com-1ecc6299db9ec823/notify-5.0.0-pre.5/src/inotify.rs:134:47

This change basically just ignores it (since we'll retry when we go through the loop again).

Copy link
Member

@0xpr03 0xpr03 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we test this somehow in CI ?

@rschoon
Copy link
Contributor Author

rschoon commented Feb 14, 2021

Probably, although getting it right seems like it might be a bit complicated. Doing it as an integration test and sending a signal to its own pid would probably be sufficient to exercise the problem, but verifying things went according to plan might be harder.

I might take a look if I have time.

src/inotify.rs Show resolved Hide resolved
Copy link
Member

@JohnTitor JohnTitor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I think it's worth mentioning this in the changelog, could you add an entry?

@0xpr03 Feel free to merge once it's done :)

@0xpr03
Copy link
Member

0xpr03 commented Feb 20, 2021

Thanks for tackling this !

@rschoon rschoon deleted the handle-interrupted-syscall branch March 24, 2021 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants