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 connection leak in Lwt client #48

Merged
merged 2 commits into from
Aug 9, 2017
Merged

Fix connection leak in Lwt client #48

merged 2 commits into from
Aug 9, 2017

Conversation

gaborigloi
Copy link
Contributor

@gaborigloi gaborigloi commented Aug 7, 2017

and update the .gitignore.

Fixes #45
Fixes #31

Close the connection that we open in the rpc function returned by
Xen_api_lwt_unix.make. Since the Xen_api_lwt_unix module itself does not
expose functions for creating, using, and closing connections, the user
would was not able to close the connections created by this rpc
function. Previously it opened a new connection for every request that
it did not close - this resulted in leaking open connections on the
client side and open file descriptors on the xapi side, which can cause
the client and/or xapi to fail after a large number of requests.

With this commit the behaviour of the Lwt bindings will be the same as
the behaviour of the Unix bindings in the xapi-client ocamlfind package
provided by xapi: a new connection will be opened and closed for each
XenAPI call.

I've added the call to disconnect to the rpc function returned by the
Lwt library, and not directly to the rpc function in lib/xen_api.ml,
because the core library in lib/ supports reusing connections for
multiple RPC calls, therefore that rpc function should not close the
connection.

Signed-off-by: Gabor Igloi <gabor.igloi@citrix.com>
Signed-off-by: Gabor Igloi <gabor.igloi@citrix.com>
@gaborigloi gaborigloi changed the title Fix connection leak in Lwt library Fix connection leak in Lwt client Aug 7, 2017
@gaborigloi
Copy link
Contributor Author

I've tested the fixed library with the repro in #45 , and it didn't leak any file descriptors in xapi, the fd count remained constant after 1000 connects & disconnects.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 7.084% when pulling 3dbf24c on gaborigloi:fix_lwt_connection_leak into 0c65bbd on xapi-project:master.

@gaborigloi
Copy link
Contributor Author

gaborigloi commented Aug 7, 2017

Apparently the Async-based client has the same issue - I'll repro it and send a PR once this one is merged.

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