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
From 9801778e5d890d099a4be3fa66e85cdf89f31084 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= <mku...@redhat.com>
Date: Mon, 10 Aug 2015 16:16:54 +0200
Subject: [PATCH] tests: fix the list comprehension leak in python 2 and free
 all nss objects

The python 2 feature/bug that binds the last value in a list
comprehension to the variable caused the nss object in the test
being bound at the time when nss.nss_shutdown was called.

This caused an ```NSPRError: (SEC_ERROR_BUSY) NSS could not shutdown.
Objects are still in use.``` exception being thrown that was not caught.
The subsequent calls to nss module corrupted the NSS database and this
led to a cascade of another errors invalidating the whole test run.

The patch works around this python 2 issue.

Fixes: https://fedorahosted.org/freeipa/ticket/5192
---
 ipatests/test_ipaserver/test_otptoken_import.py | 31 ++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/ipatests/test_ipaserver/test_otptoken_import.py b/ipatests/test_ipaserver/test_otptoken_import.py
index c8818a01ea4d4bad4b4a0f656357f7b32407b86a..6f383ea7a3f511733aacef20eee54440bd513ec6 100644
--- a/ipatests/test_ipaserver/test_otptoken_import.py
+++ b/ipatests/test_ipaserver/test_otptoken_import.py
@@ -19,7 +19,6 @@
 
 import os
 import sys
-import nose
 from nss import nss
 from ipalib.x509 import initialize_nss_database
 
@@ -27,6 +26,7 @@ from ipaserver.install.ipa_otptoken_import import PSKCDocument, ValidationError
 
 basename = os.path.join(os.path.dirname(__file__), "data")
 
+
 class test_otptoken_import(object):
 
     def teardown(self):
@@ -34,6 +34,7 @@ class test_otptoken_import(object):
 
     def test_figure3(self):
         doc = PSKCDocument(os.path.join(basename, "pskc-figure3.xml"))
+        t = None
         assert doc.keyname is None
         assert [(t.id, t.options) for t in doc.getKeyPackages()] == \
             [(u'12345678', {
@@ -44,29 +45,37 @@ class test_otptoken_import(object):
                 'ipatokenotpdigits': 8,
                 'type': u'hotp',
                 })]
+        del t
 
     def test_figure4(self):
         doc = PSKCDocument(os.path.join(basename, "pskc-figure4.xml"))
+        t = None
         assert doc.keyname is None
         try:
             [(t.id, t.options) for t in doc.getKeyPackages()]
-        except ValidationError: # Referenced keys are not supported.
+        except ValidationError:  # Referenced keys are not supported.
             pass
         else:
             assert False
+        finally:
+            del t
 
     def test_figure5(self):
         doc = PSKCDocument(os.path.join(basename, "pskc-figure5.xml"))
+        t = None
         assert doc.keyname is None
         try:
             [(t.id, t.options) for t in doc.getKeyPackages()]
-        except ValidationError: # PIN Policy is not supported.
+        except ValidationError:  # PIN Policy is not supported.
             pass
         else:
             assert False
+        finally:
+            del t
 
     def test_figure6(self):
         nss.nss_init_nodb()
+        t = None
         try:
             doc = PSKCDocument(os.path.join(basename, "pskc-figure6.xml"))
             assert doc.keyname == 'Pre-shared-key'
@@ -80,10 +89,12 @@ class test_otptoken_import(object):
                     'ipatokenotpdigits': 8,
                     'type': u'hotp'})]
         finally:
+            del t
             nss.nss_shutdown()
 
     def test_figure7(self):
         nss.nss_init_nodb()
+        t = None
         try:
             doc = PSKCDocument(os.path.join(basename, "pskc-figure7.xml"))
             assert doc.keyname == 'My Password 1'
@@ -95,14 +106,16 @@ class test_otptoken_import(object):
                     'ipatokenserial': u'987654321',
                     'ipatokenotpdigits': 8,
                     'type': u'hotp'})]
+
         finally:
+            del t
             nss.nss_shutdown()
 
     def test_figure8(self):
         nss.nss_init_nodb()
         try:
             doc = PSKCDocument(os.path.join(basename, "pskc-figure8.xml"))
-        except NotImplementedError: # X.509 is not supported.
+        except NotImplementedError:  # X.509 is not supported.
             pass
         else:
             assert False
@@ -113,7 +126,7 @@ class test_otptoken_import(object):
         nss.nss_init_nodb()
         try:
             doc = PSKCDocument(os.path.join(basename, "pskc-invalid.xml"))
-        except ValueError: # File is invalid.
+        except ValueError:  # File is invalid.
             pass
         else:
             assert False
@@ -122,18 +135,22 @@ class test_otptoken_import(object):
 
     def test_mini(self):
         nss.nss_init_nodb()
+        t = None
         try:
             doc = PSKCDocument(os.path.join(basename, "pskc-mini.xml"))
             [(t.id, t.options) for t in doc.getKeyPackages()]
-        except ValidationError: # Unsupported token type.
+
+        except ValidationError:  # Unsupported token type.
             pass
         else:
             assert False
         finally:
+            del t
             nss.nss_shutdown()
 
     def test_full(self):
         nss.nss_init_nodb()
+        t = None
         try:
             doc = PSKCDocument(os.path.join(basename, "full.xml"))
             assert [(t.id, t.options) for t in doc.getKeyPackages()] == \
@@ -152,5 +169,7 @@ class test_otptoken_import(object):
                     'ipatokenotpdigits': 8,
                     'type': u'hotp',
                 })]
+
         finally:
+            del t
             nss.nss_shutdown()
-- 
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