pyldap / pyldap

THIS FORK IS DEPRECATED; development moved to python-ldap
https://github.com/python-ldap/python-ldap
Other
105 stars 34 forks source link

Move "sys.exc_info()" back to start of try block to fix py2 behavior #135

Closed encukou closed 6 years ago

encukou commented 6 years ago

Instead, del the temporaty variables explicitly to prevent cycles

This fixes commit d16bccbf41e72fb934113ead94670c864b000dd5.

See https://github.com/pyldap/pyldap/pull/133#issuecomment-344883573

darkrain42 commented 6 years ago

I'm not sure that fix works. I wouldn't expect control to return from reraise, so the deletions would never run. You might need to wrap the reraise in a try / finally?

Separately, dunno whether this is worth adding or not, but I'd written this while looking at a local fix/patch for #124.

diff --git a/Tests/t_ldapobject.py b/Tests/t_ldapobject.py
index 8d9ea37..b44b182 100644
--- a/Tests/t_ldapobject.py
+++ b/Tests/t_ldapobject.py
@@ -299,6 +299,15 @@ class Test01_SimpleLDAPObject(SlapdTestCase):
         l = self.ldap_object_class(self.server.ldapi_uri)
         l.sasl_external_bind_s(authz_id=authz_id)
         self.assertEqual(l.whoami_s(), authz_id.lower())
+
+    def test_timeout(self):
+        l = self.ldap_object_class(self.server.ldap_uri)
+
+        m = l.search_ext(self.server.suffix, ldap.SCOPE_SUBTREE, '(objectClass=*)')
+        l.abandon(m)
+
+        with self.assertRaises(ldap.TIMEOUT):
+            result = l.result(m, timeout=0.1)

  class Test02_ReconnectLDAPObject(Test01_SimpleLDAPObject):
tiran commented 6 years ago

The del commands are never executed because reraise raises an exception. You have to wrap it in a try-finally block:

try:
    reraise(exc_type, exc_value, exc_traceback)
finally:
    exc_type = exc_value = exc_traceback = None
encukou commented 6 years ago

Gah. Never fix issues in a hurry :/

Thanks for the suggestions. Here's @tiran's fix, and I added @darkrain42's test for good measure. More tests are always better.

tiran commented 6 years ago

Do you have time to submit the fix upstream?

encukou commented 6 years ago

It's already fixed there; this change is porting that fix to Python 3.

encukou commented 6 years ago

Oh, right; you mean the test. I'll put it on my TODO list, but I can't guarantee I'll get to it in reasonable time.