-
Notifications
You must be signed in to change notification settings - Fork 22
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
Stop TCP interfaces and RPC connections on shutdown #3307
Conversation
source/agora/network/VibeManager.d
Outdated
@@ -59,13 +59,23 @@ shared static this () | |||
/// And implementation of `agora.network.Manager : NetworkManager` using Vibe.d | |||
public final class VibeNetworkManager : NetworkManager | |||
{ | |||
private RPCClient!(agora.api.Validator.API)[] rpc_clients; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cant we just close()
the socket in RPCClient
dtor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think all interface and connections should be closed before invoking node's shutdown procedure. But this might cause holding an unnecessary reference for cases where a connection is banned . I will think a better approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated it with new approach, ready to review
Codecov Report
@@ Coverage Diff @@
## v0.x.x #3307 +/- ##
==========================================
+ Coverage 87.88% 88.92% +1.04%
==========================================
Files 164 165 +1
Lines 16975 17108 +133
==========================================
+ Hits 14918 15213 +295
+ Misses 2057 1895 -162
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Towards a better shutdown procedure
This will reduce the number of eventcore leaks during shutdown, only DNS UDP connection will be remaining as leaking.
Part of #3158
depends on #3311 and bosagora/localrest#6