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

Fix some shutdown issues in the web plugin #2860

Merged
merged 5 commits into from Jan 20, 2023
Merged

Conversation

lava
Copy link
Member

@lava lava commented Jan 13, 2023

This PR fixes 2/3 shutdown issues I found in the web plugin; in particular the web plugin now shuts down correctly when run as a separate command when receiving a signal and when detecting a node shutdown.

Note that it does not address the situation where web server is passed as a start command to vast start.

These are extracted from 'server_command.cpp', where
they'll be removed in a subsequent commit.
@lava lava force-pushed the story/sc-38788/web-shutdown branch 2 times, most recently from ec34a59 to d0a13fa Compare January 13, 2023 12:37
@lava lava added the bug Incorrect behavior label Jan 13, 2023
@lava lava force-pushed the story/sc-38788/web-shutdown branch from 41e4ea5 to d0a13fa Compare January 13, 2023 17:54
@Dakostu Dakostu self-requested a review January 16, 2023 09:34
This commit moves the web server into a separate thread
so that the main thread can wait for DOWN messages and
signal notifications in the meantime.

It also fixes an issue where the spawned handlers would
ignore an `exit_msg` with a default-constructed caf::error;
we now send a user_shutdown message instead.
@lava lava force-pushed the story/sc-38788/web-shutdown branch from d0a13fa to a92e829 Compare January 16, 2023 09:49
Copy link
Member

@Dakostu Dakostu left a comment

Choose a reason for hiding this comment

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

My original SIGINT sending issue has been fixed.

@Dakostu
Copy link
Member

Dakostu commented Jan 18, 2023

@lava
Does this warrant a changelog entry?

@lava lava enabled auto-merge January 20, 2023 11:25
@lava lava merged commit 4d19c10 into master Jan 20, 2023
@lava lava deleted the story/sc-38788/web-shutdown branch January 20, 2023 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect behavior
Projects
None yet
3 participants