[Freeipa-devel] [patch 0010] Python list comprehension leak breaking the test execution

[Milan Kubík]

On 08/11/2015 09:08 AM, Jan Cholasta wrote:
> On 11.8.2015 09:00, Milan Kubík wrote:
>
>> On 08/10/2015 06:22 PM, Milan Kubík wrote:
>>
>>> On 08/10/2015 06:02 PM, Milan Kubík wrote:
>>>
>>>> On 08/10/2015 05:54 PM, Jan Cholasta wrote:
>>>>
>>>>> On 10.8.2015 17:43, Milan Kubík wrote:
>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>>
>>>>>>
>>>>>> this patch fixes problem described in the ticket [1]
>>>>>>
>>>>>> that caused the test run to fail completely at every other or so run.
>>>>>>
>>>>>> I took the liberty to fix most of the pep8 issues while I was at it.
>>>>>>
>>>>>>
>>>>>>
>>>>>> Thanks to Jan Cholasta for help with identifying this one.
>>>>>>
>>>>>
>>>>>
>>>>> IMO this would be more robust:
>>>>>
>>>>>
>>>>>
>>>>>     t = None
>>>>>
>>>>>     try:
>>>>>
>>>>>         ...
>>>>>
>>>>>     finally:
>>>>>
>>>>>         del t
>>>>>
>>>>>         nss.nss_shutdown()
>>>>>
>>>>>
>>>>>
>>>>> By assigning a value to the variable at the beginning you make sure
>>>>>
>>>>> that the del statement will not fail.
>>>>>
>>>>>
>>>>>
>>>>> Honza
>>>>>
>>>>>
>>>>>
>>>> Thanks for the idea. It also removed the version check.
>>>>
>>>> Updated patch attached.
>>>>
>>>>
>>>>
>>>> Milan
>>>>
>>>>
>>>>
>>>>
>>>>
>>> Self NACK.
>>>
>>>
>>>
>>> I have updated the fix for all of the leaks in that class. It seems to
>>>
>>> corrupt the database even when no init/shutdown was called. Just not
>>>
>>> so often.
>>>
>>>
>>>
>>> Milan
>>>
>>>
>>>
>>>
>>>
>> NACK again. The problem is the reference counting. Even with this patch,
>>
>> there seems to be at least one reference left after 't' is deleted and
>>
>> nss.nss_shutdown races with the garbage collector.
>>
>
>
> Doesn't the PKCDDocument object also reference some NSS objects? It
> might be worth trying to delete it manually before nss_shutdown as well.
>
>
>
>
>
Yes, this patch doesn't work. There are still some references left.

The problem may be on multiple places in here [1].

There may be a reference still bound to the doc label.
Another problem is that python 2 code:

[x for x in [123, 456]]

creates 2 references to 456 as the list used in the assert lives for 
some time
before it is garbage collected even though it is not reachable,
holding a reference to the object labeled as t.

I don't know how nss counts the references to its objects but I think we 
should
delete all the objects that use/are used by nss explicitly. This means 
assigning the list
produced by the list comprehension a name as well and the delete it when 
it is not needed.

I'll send the patch shortly.

[1]: https://paste.fedoraproject.org/253748/92783071/

Milan