[ovirt-devel] FD leak while initializing vdsm

[Nir Soffer]

On Wed, May 25, 2016 at 12:18 PM, Martin Sivak <msivak at redhat.com> wrote:
>> but it should
>> not try to implement __del__ - we had lot of trouble with such code in vdsm.
>
> Del is perfectly fine if done properly. Python objects should really
> clean after themselves. It is common to abstract the user from the low
> level networking code. For example all the SocketServer or xmlrpclib
> abstractions close the sockets automatically.

$ grep __del__ /usr/lib64/python2.7/SocketServer.py
$ grep __del__ /usr/lib64/python2.7/xmlrpclib.py
$ grep __del__ /usr/lib64/python2.7/SimpleXMLRPCServer.py

> The same is true about
> file objects (closed when garbage collected).

This is implemented as C extension, not using __del__.

>
> Context manager can (and should if possible) be used to handle
> exceptions properly and to allow better control, but the default
> behavior should still close the resources during __del__ too.

I disagree, __del__ cannot be implemented properly on python 2.
It is possible to close stuff automatically only in a c extension.
Trying to implement will lead to endless troubles.

Nir

>
> Martin
>
>
> On Mon, May 23, 2016 at 2:57 PM, Nir Soffer <nsoffer at redhat.com> wrote:
>> On Mon, May 23, 2016 at 3:46 PM, Martin Sivak <msivak at redhat.com> wrote:
>>>> Explicit close is required.
>>>>
>>>> You should ensure that all resources are released using try finally.
>>>
>>> Not according to the code Simone found. The close is called by the
>>> __del__ method explicitly. And Python is no C, we should really behave
>>> "nice" by default.
>>
>> No, the code you found is misguided attempt and should be deleted from vdsm.
>>
>> The jsonrpc client can provide a context manger to help you close it,
>> but it should
>> not try to implement __del__ - we had lot of trouble with such code in vdsm.
>>
>> You must close the client explicitly when you are done.
>>
>> Nir
>>
>>>
>>> Martin
>>>
>>> On Mon, May 23, 2016 at 2:40 PM, Nir Soffer <nsoffer at redhat.com> wrote:
>>>> On Mon, May 23, 2016 at 11:15 AM, Martin Sivak <msivak at redhat.com> wrote:
>>>>> Hi,
>>>>>
>>>>>> I know that we reconnect several times during hosted engine process. Do we
>>>>>> close client when it is not used anymore?
>>>>>
>>>>> No, we are not closing it according to Simone, shouldn't it be
>>>>> released automatically? We are using Python after all.. explicit close
>>>>> is not exactly common there.
>>>>
>>>> Explicit close is required.
>>>>
>>>> You should ensure that all resources are released using try finally.
>>>>
>>>> Nir
>>>>
>>>>>
>>>>> Martin
>>>>>
>>>>> On Mon, May 23, 2016 at 8:55 AM, Piotr Kliczewski <pkliczew at redhat.com> wrote:
>>>>>> Sandro,
>>>>>>
>>>>>> I know that we reconnect several times during hosted engine process. Do we
>>>>>> close client when it is not used anymore?
>>>>>>
>>>>>> Please provide lsof for the process and the log.
>>>>>>
>>>>>> Thanks,
>>>>>> Piotr
>>>>>>
>>>>>> On Mon, May 23, 2016 at 8:42 AM, Sandro Bonazzola <sbonazzo at redhat.com>
>>>>>> wrote:
>>>>>>>
>>>>>>> MainThread::WARNING::2016-05-23
>>>>>>> 07:09:38,629::hosted_engine::480::ovirt_hosted_engine_ha.agent.hosted_engine.HostedEngine::(start_monitoring)
>>>>>>> Unexpected error
>>>>>>> Traceback (most recent call last):
>>>>>>>   File
>>>>>>> "/usr/lib/python2.7/site-packages/ovirt_hosted_engine_ha/agent/hosted_engine.py",
>>>>>>> line 444, in start_monitoring
>>>>>>>     self._initialize_vdsm()
>>>>>>>   File
>>>>>>> "/usr/lib/python2.7/site-packages/ovirt_hosted_engine_ha/agent/hosted_engine.py",
>>>>>>> line 635, in _initialize_vdsm
>>>>>>>     timeout=envconstants.VDSCLI_SSL_TIMEOUT
>>>>>>>   File
>>>>>>> "/usr/lib/python2.7/site-packages/ovirt_hosted_engine_ha/lib/util.py", line
>>>>>>> 187, in connect_vdsm_json_rpc
>>>>>>>     requestQueue=requestQueue,
>>>>>>>   File "/usr/lib/python2.7/site-packages/vdsm/jsonrpcvdscli.py", line 222,
>>>>>>> in connect
>>>>>>>     responseQueue)
>>>>>>>   File "/usr/lib/python2.7/site-packages/vdsm/jsonrpcvdscli.py", line 212,
>>>>>>> in _create
>>>>>>>     lazy_start=False)
>>>>>>>   File "/usr/lib/python2.7/site-packages/yajsonrpc/stompreactor.py", line
>>>>>>> 576, in StandAloneRpcClient
>>>>>>>     reactor = Reactor()
>>>>>>>   File "/usr/lib/python2.7/site-packages/yajsonrpc/betterAsyncore.py",
>>>>>>> line 200, in __init__
>>>>>>>     self._wakeupEvent = AsyncoreEvent(self._map)
>>>>>>>   File "/usr/lib/python2.7/site-packages/yajsonrpc/betterAsyncore.py",
>>>>>>> line 164, in __init__
>>>>>>>     map=map
>>>>>>>   File "/usr/lib64/python2.7/asyncore.py", line 650, in __init__
>>>>>>>     self.set_file(fd)
>>>>>>>   File "/usr/lib64/python2.7/asyncore.py", line 657, in set_file
>>>>>>>     self.socket = file_wrapper(fd)
>>>>>>>   File "/usr/lib64/python2.7/asyncore.py", line 616, in __init__
>>>>>>>     self.fd = os.dup(fd)
>>>>>>> OSError: [Errno 24] Too many open files
>>>>>>>
>>>>>>> Simone, Rafael, Piotr, Martin, can you please investigate?
>>>>>>>
>>>>>>> vdsm-yajsonrpc-4.18.0-16.git51df339.el7.centos.noarch
>>>>>>>
>>>>>>> ovirt-hosted-engine-ha-2.0.0-0.2.master.20160520143206.20160520143149.gita012f18.el7.noarch
>>>>>>>
>>>>>>> --
>>>>>>> Sandro Bonazzola
>>>>>>> Better technology. Faster innovation. Powered by community collaboration.
>>>>>>> See how it works at redhat.com
>>>>>>
>>>>>>
>>>>> _______________________________________________
>>>>> Devel mailing list
>>>>> Devel at ovirt.org
>>>>> http://lists.ovirt.org/mailman/listinfo/devel