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

Cleaner daemon stopping #4560

Conversation

pwojcikdev
Copy link
Contributor

@pwojcikdev pwojcikdev commented Apr 14, 2024

Current daemon stopping logic is unnecessarily convoluted and relies on io context being stopped first, which is problematic. This should unblock #4523. I expect this will also make it easier to move io context into the node.

There are no CI tests that ensure proper daemon operation, but by testing it manually it seems to work fine. I'll see if I can implement such test and submit it in another PR.

[2024-04-14 19:10:39.736] [daemon] [warning] Interrupt signal received (SIGTERM), stopping...
[2024-04-14 19:10:39.736] [daemon] [info] Stopping...
[2024-04-14 19:10:39.736] [rpc] [error] Error accepting RPC connection: Operation canceled
[2024-04-14 19:10:39.736] [node] [info] Node stopping...
[2024-04-14 19:10:39.768] [tcp_listener] [error] Unable to accept connection: Operation canceled (0.0.0.0)
[2024-04-14 19:10:39.769] [tcp_listener] [error] Acceptor is not open
[2024-04-14 19:10:39.778] [upnp] [warning] UPnP shutdown TCP port mapping failed: 714 (NoSuchEntryInArray)
[2024-04-14 19:10:39.972] [daemon] [info] Daemon stopped

nano/lib/signal_manager.cpp Outdated Show resolved Hide resolved
nano/nano_node/daemon.cpp Outdated Show resolved Hide resolved
@pwojcikdev pwojcikdev force-pushed the networking-fixes/daemon-latch-rebased branch from 1fa4b3a to 29e7218 Compare April 14, 2024 20:17
@pwojcikdev pwojcikdev merged commit 4d66da7 into nanocurrency:develop Apr 17, 2024
23 of 26 checks passed
nano::signal_handler_impl = [&ioc] () {
nano::signal_manager sigman;

auto signal_handler = [&ioc] (int signum) {
ioc.stop ();
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks wrong, although I do not know if we are still using the load_test.

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