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

[Milan Kubík]

On 08/11/2015 01:46 PM, Christian Heimes wrote:
> On 2015-08-11 09:46, Milan Kubík wrote:
>> 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.
> In Python 2 the list comprehension leaks the internal loop variable. It
> was an oversight in the design of list comprehensions. This has been
> fixed in Python 3. You can work around the problem with a generator
> expression inside list()
>
>     list(x for x in [123, 456])
>
> is equivalent to
>
>    [x for x in [123, 456]]
>
> except it doesn't leak x to the outer scope.
>
> If you suspect an issue related to cyclic GC, you can force a garbage
> collector run with gc.collect().
>
> Christian
>
>
Hi Christian, I already use the generator expression as Martin suggested 
earlier today. Together with the decorator proposed by Jan. I had some 
trouble with the test teardown method, but it seems I had a minor 
problem in my environment as there was missing ~/.ipa/alias directory in 
the test run.

On the other hand, while not changing the job on jenkins I wonder why it 
worked sometimes in this env.

Milan