On 08/11/2015 03:23 PM, Milan Kubík wrote:
On 08/11/2015 09:53 AM, Jan Cholasta wrote:
On 11.8.2015 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.

I don't know how nss counts the references to its objects but I think we
should ow I think
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/

Given an assumption that all objects referenced only by local variables are deleted when a function returns, maybe this can be solved with:

    def nss_initialized(func):
        def decorated(*args, **kwargs):
            nss.nss_init_nodb()
            try:
                func(*args, **kwargs)
            finally:
                nss.nss_shutdown()
        return decorated

    ...

    class test_class(...):
        @nss_initialized
        def test_method(self):
            ...

(OTOH and untested)

It seems that NSS decided the assumption doesn't hold. [1]
Plus, the decorator seems to change the execution order of the test cases.

[1]: https://paste.fedoraproject.org/253846/39299083/

Milan

Hi list,

since I wasn't successful at solving this problem, I revert to the original plan and propose to take the otp import test out of the execution. The attached patch
does this.

By no means is this a solution to the problem. Therefore, I think the ticket should remain open. The priority of blocker can be dropped, though. I think 'critical' should be used.

Cheers,
Milan

From d2075af69de40910721f14cdc4cf56d20c4b121a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= <mku...@redhat.com>
Date: Mon, 17 Aug 2015 09:06:24 +0200
Subject: [PATCH] ipatests: Take otptoken import test out of execution

The issue reported in ticket [1] hasn't been solved yet.

This patch prevents the test cases for OTP import being run.
The change is intended as a *temporary* workaround until
proper fix for the issue is introduced.

[1] https://fedorahosted.org/freeipa/ticket/5192
---
 ipatests/test_ipaserver/test_otptoken_import.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ipatests/test_ipaserver/test_otptoken_import.py b/ipatests/test_ipaserver/test_otptoken_import.py
index c8818a01ea4d4bad4b4a0f656357f7b32407b86a..798f721f5f921da3648e9b285b4f50e18770f448 100644
--- a/ipatests/test_ipaserver/test_otptoken_import.py
+++ b/ipatests/test_ipaserver/test_otptoken_import.py
@@ -20,6 +20,7 @@
 import os
 import sys
 import nose
+import pytest
 from nss import nss
 from ipalib.x509 import initialize_nss_database
 
@@ -27,6 +28,7 @@ from ipaserver.install.ipa_otptoken_import import PSKCDocument, ValidationError
 
 basename = os.path.join(os.path.dirname(__file__), "data")
 
+@pytest.mark.skipif(True, reason="Causes NSS errors. Ticket 5192")
 class test_otptoken_import(object):
 
     def teardown(self):
-- 
2.5.0

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to