Change in vdsm[master]: xmlrpc: fd leak on centos7

nsoffer at redhat.com nsoffer at redhat.com
Fri Apr 3 23:21:33 UTC 2015


Nir Soffer has posted comments on this change.

Change subject: xmlrpc: fd leak on centos7
......................................................................


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/39506/1/lib/vdsm/sslutils.py
File lib/vdsm/sslutils.py:

Line 54:         self.connection.shutdown(socket.SHUT_RDWR)
Line 55:         self.connection.close()
Line 56:         # if we do not close socket we get ssl protocol shutdown
Line 57:         # but fd is not released
Line 58:         self.connection.socket.close()
We already fixed similar issue in https://gerrit.ovirt.org/33982

M2Crypto is closing the socket in __del__ - if we keep a reference to the connection, or to an object holding a reference to the connection, the socket will never be closed.

So I would look for leak of SSLSocket object, or a leak of an object keeping a reference to it. Fixing this leak will also close the socket.

For long term we must fix M2Crypto so socket will be closed when the shutdown is finished, and not in __del__. Tying resource allocation and memory management is bad idea.
Line 59: 
Line 60:     def fileno(self):
Line 61:         return self.connection.fileno()
Line 62: 


-- 
To view, visit https://gerrit.ovirt.org/39506
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I705b0cf39937fe8d175305ad8ea8ad615f3ffb49
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list