Skip to content

Handle Ctrl+C exit gracefully.#140

Merged
Bash-09 merged 2 commits intomainfrom
Bash/GracefulExit
Apr 28, 2024
Merged

Handle Ctrl+C exit gracefully.#140
Bash-09 merged 2 commits intomainfrom
Bash/GracefulExit

Conversation

@Bash-09
Copy link
Copy Markdown
Contributor

@Bash-09 Bash-09 commented Apr 28, 2024

Previously I just let Ctrl+C kill the program, but there is a possibility that poor timing could lead to data loss if settings or records have not been written back to disk yet. This branch installs a handler for Ctrl+C which gives the program a chance to save any persistent data before exiting.

It works on Linux but I have not tested it on Windows, although the tokio::signal module claims to be cross-platform so I imagine it will probably work as expected there too.

@Bash-09 Bash-09 requested a review from megascatterbomb April 28, 2024 06:44
@Bash-09 Bash-09 mentioned this pull request Apr 28, 2024
@megascatterbomb
Copy link
Copy Markdown
Contributor

Appears to work correctly during a normal ctrl-c

image

I tested a ctrl-c during startup and it doesn't appear to be handled. As long as the playerlist isn't being accessed at this time it should be okay?

image

@Bash-09
Copy link
Copy Markdown
Contributor Author

Bash-09 commented Apr 28, 2024

Hm yeah I did only setup the handler right before it enters the main loop, which I guessed would be fine, but it would probably wouldn't hurt to set it up earlier anyways.

I've pushed a commit that just moves it earlier before anything that could touch the settings and records, but it is after the masterbase check so that can still be cancelled early.

Copy link
Copy Markdown
Contributor

@megascatterbomb megascatterbomb left a comment

Choose a reason for hiding this comment

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

LGTM

@Bash-09 Bash-09 merged commit b69537a into main Apr 28, 2024
@Bash-09 Bash-09 deleted the Bash/GracefulExit branch April 28, 2024 07:19
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.

2 participants