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

feat: Handling regular signals sent to cargo-shuttle on Windows #1077

Merged
merged 5 commits into from
Jul 31, 2023

Conversation

Shubham8287
Copy link
Contributor

@Shubham8287 Shubham8287 commented Jul 9, 2023

Description of change

Gracefully handling windows signal to interrupt local run for #872.
/claim #872

How has this been tested? (if applicable)

Tested by running on windows, with Ctrl+C

@Shubham8287
Copy link
Contributor Author

Right now Implemented handler for Ctrl+C only, it can be extended to other ones. Would love to get some input on which one to add from https://docs.rs/tokio/latest/tokio/signal/windows/index.html.
Also, Signal handler looks is mostly repeated for Unix and windows and between different handlers. Will try to have signal handling as wrapper or something.

@iulianbarbu
Copy link
Contributor

I think we need to handle all of them: https://learn.microsoft.com/en-us/windows/console/setconsolectrlhandler#remarks. A default handler for all these signals calls ExitProcess.

@Shubham8287
Copy link
Contributor Author

Added all the handlers, please review

@iulianbarbu
Copy link
Contributor

Seems like tests are failing because of cargo fmt. Can you fix it?

@Shubham8287
Copy link
Contributor Author

@iulianbarbu passed all checks now.

@iulianbarbu
Copy link
Contributor

Thanks @Shubham8287 ! We just need to do some manual testing on a windows machine. We'll follow up after.

Copy link
Contributor

@iulianbarbu iulianbarbu left a comment

Choose a reason for hiding this comment

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

Testing worked well on my side @Shubham8287 ! thanks.

@oddgrd oddgrd merged commit c3c5d4c into shuttle-hq:main Jul 31, 2023
iulianbarbu added a commit that referenced this pull request Aug 2, 2023
* Added Ctlc+C handler for windows

* add all handlers

* fix linting

---------

Co-authored-by: Shubham Mishra <shubmishra@microsoft.com>
Co-authored-by: Iulian Barbu <14218860+iulianbarbu@users.noreply.github.com>
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.

5 participants