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

Investigate upstream Vibe.d issue: Handle leak #1136

Closed
Geod24 opened this issue Aug 24, 2020 · 2 comments
Closed

Investigate upstream Vibe.d issue: Handle leak #1136

Geod24 opened this issue Aug 24, 2020 · 2 comments
Assignees
Labels
C. Network Communication An issue which is related to network communication type-bug Things don't work as they were intended to, or the way they were intended to work doesn't make sense
Milestone

Comments

@Geod24
Copy link
Collaborator

Geod24 commented Aug 24, 2020

We're currently seeing the following output in the integration tests:

node-7_1_3e996a01b79d | 2020-08-23T14:44:18.638919694Z Warning (thread: main): leaking eventcore driver because there are still active handles
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.638924687Z    FD 10 (streamListen)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.638929101Z    FD 12 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.638933411Z    FD 13 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.638937809Z    FD 14 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.638942036Z    FD 15 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.638946347Z    FD 16 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.638951363Z    FD 17 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.638955579Z    FD 18 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.638959669Z    FD 19 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.638980753Z    FD 20 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.638985588Z    FD 21 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.638989727Z    FD 22 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.638993792Z    FD 23 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.638997937Z    FD 24 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.639001990Z    FD 25 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.639006007Z    FD 26 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.639010869Z    FD 27 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.639015242Z    FD 28 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.639019546Z    FD 29 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.639023989Z    FD 30 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.639028130Z    FD 31 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.639032640Z    FD 32 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.639036978Z    FD 33 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.639041578Z    FD 34 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.639067064Z    FD 35 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.639072500Z    FD 36 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.639076984Z    FD 37 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.639081561Z    FD 38 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.639086015Z    FD 39 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.639090539Z    FD 40 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.639094719Z    FD 41 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.639105485Z    FD 42 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.639109921Z    FD 43 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.639114316Z    FD 44 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.639118739Z    FD 46 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.639123226Z    FD 47 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.639127643Z    FD 48 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.639150530Z    FD 49 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.639154919Z    FD 50 (streamSocket)
node-7_1_3e996a01b79d | 2020-08-23T14:44:18.639159332Z    FD 53 (streamSocket)

It would be good to take a look into the issue. Some upstream discussion: vibe-d/vibe.d#2245

@Geod24 Geod24 added the type-bug Things don't work as they were intended to, or the way they were intended to work doesn't make sense label Aug 24, 2020
@Geod24 Geod24 added this to the 2. Validator milestone Aug 24, 2020
@ferencdg ferencdg self-assigned this Aug 24, 2020
@ferencdg
Copy link
Contributor

ferencdg commented Aug 25, 2020

quick update on the issue:

  1. streamListen
    I managed to solve this by issuing stopListen on the HttpListener object as the discussion on the vibe.d issue tracker suggested
  2. streamSocket(this issue was NOT reported on the vibe.d tracker)
    I managed to reproduce it in 2 different ways:

2.1 Reproduce issue of streamSocket

void main()
{
    auto settings = new HTTPServerSettings;
    settings.bindAddresses = ["127.0.0.1"];
	settings.port = 8080;
    auto router = new URLRouter;
	router.get("/weather", &weather);
    HTTPListener httpListener = listenHTTP(settings, router);

	runApplication();
    httpListener.stopListening;
}

2.1.1 start up the server above
2.1.2 press CTRL+C(the server will not exit immediately)
2.1.3 send curl -v 127.0.0.1:8080/weather from the command line
when the server finally exits it will report streamSocket error
if you change the order of 2.1.2 and 2.1.3, then there is no error => socets after CTRL+C are not cleaned by vibe.d

2.2 Reproduce issue of streamSocket

2.2.1
start up agora and make sure the config files has hosts that are unreacheble with ping,for example
network:

2.2.2 you will see some connection attempts like this:
image
2.2.3 when you press CTRL+C, those sockets in SYN states are reported as leaking streamSockets
2.2.4 I verified in the vide.d code that we register this new sockets into our internal datastructures, before we even manage to fully connect to the remote node

if you let the sockets connect to hosts successfully, then they are cleaned up after CTRL+C

it doesn't seem straightforward to fix either 2.1 or 2.2 without delving into vibe.d code very heavily. Vibe.d does register signal handlers, so it does catch SIGINT and SIGTERM, and attempts to clean up after itself, it just doesn't succeed in it.

@ferencdg
Copy link
Contributor

ferencdg commented Aug 25, 2020

and easier way to reproduce 2.2 is this:

void main()
{
    runTask({
        requestHTTP("http://www.index.hu",
            (scope req) {
                req.method = HTTPMethod.GET;
            },
            (scope res) {
                logInfo("Response: %s", res.bodyReader.readAllUTF8());
            }
        );

        requestHTTP("http://192.168.1.42:2828", // make sure this is not reachable by ping
            (scope req) {
                req.method = HTTPMethod.GET;
            },
            (scope res) {
                logInfo("Response: %s", res.bodyReader.readAllUTF8());
            }
        );
	});
  
	runApplication();
}

compile with "versions": [ "VibeHighEventPriority" ]

  1. start the app
  2. CTRL+C
    you will see the the leaking streamSocket exception ONLY ONCE, this suggests that vibe.d only mishandles/leaks connections that are in SYN state

linked0 pushed a commit to linked0/agora that referenced this issue Aug 27, 2020
Manually call `stopListening()` on the `HTTPListener` after node shutdown to avoid leaking handles.

Fixes bosagora#1136
@bpalaggi bpalaggi added the C. Network Communication An issue which is related to network communication label Sep 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C. Network Communication An issue which is related to network communication type-bug Things don't work as they were intended to, or the way they were intended to work doesn't make sense
Projects
None yet
Development

No branches or pull requests

3 participants