On 04/24/2015 07:19 AM, Jan Cholasta wrote:
Dne 23.4.2015 v 22:18 Nathaniel McCallum napsal(a):
On Thu, 2015-04-23 at 14:12 +0200, Petr Vobornik wrote:
On 04/23/2015 12:24 PM, Petr Vobornik wrote:
If unbind was called when disconnected it raised:
    AttributeError: 'NoneType' object has no attribute 'unbind_s'

AttributeError is not a public error and therefore it prevented
ldap2.destroy_connection() to be called multiple times.

fixes:
https://fedorahosted.org/freeipa/ticket/4991

Note: this issue also prevented rpcserver.change_password from
working.
Therefore I think that there might have been an error in recent
ipaldap
refactoring and if #4991 was not run on master then there might
have
been other issue, which probably have been fixed by the
refactoring.


After discussion with Honza, the approach was changed.

Also I've added patch which removes unnecessary incorrect code which
revealed the regression.

Additional testing shows that these patches actually don't fix the
original issue of #4991. See
https://fedorahosted.org/freeipa/ticket/4991#comment:4

0823 - ACK
0824 - ACK

Nathaniel


I would prefer if the connection was closed manually in patch 824, IMO
it is a good practice to release resources once you are done with them
just in time, and I don't think you can always trust the automatic
disconnect at the end of request.


Changed (also in user-status command).

--
Petr Vobornik
From 246d0b070924ca3a06168b583229e94bd68917f9 Mon Sep 17 00:00:00 2001
From: Petr Vobornik <pvobo...@redhat.com>
Date: Fri, 24 Apr 2015 13:09:47 +0200
Subject: [PATCH] use Connectible.disconnect() instead of .destroy_connection()

Destroy connection is an internal function of Connectible and therefore
it should not be used directly.

https://fedorahosted.org/freeipa/ticket/4991
---
 ipalib/plugins/user.py | 2 +-
 ipaserver/rpcserver.py | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/ipalib/plugins/user.py b/ipalib/plugins/user.py
index 2166251a23631270d7a639f2e1cbece0fdcbfd2f..3b1afca0432e070401ac2fa1d36c5dec59fbaf9c 100644
--- a/ipalib/plugins/user.py
+++ b/ipalib/plugins/user.py
@@ -1015,7 +1015,7 @@ class user_status(LDAPQuery):
                 count += 1
 
             if host != api.env.host:
-                other_ldap.destroy_connection()
+                other_ldap.disconnect()
 
         return dict(result=entries,
                     count=count,
diff --git a/ipaserver/rpcserver.py b/ipaserver/rpcserver.py
index 2f771a0d15a68f17498a617bdeb44f89a3a3faee..5158c42123b23642046479096b96963f593f48a7 100644
--- a/ipaserver/rpcserver.py
+++ b/ipaserver/rpcserver.py
@@ -1077,7 +1077,7 @@ class change_password(Backend, HTTP_Status):
                 message = "Password was changed."
             finally:
                 if conn.isconnected():
-                    conn.destroy_connection()
+                    conn.disconnect()
 
         self.info('%s: %s', status, message)
 
@@ -1182,7 +1182,7 @@ class sync_token(Backend, HTTP_Status):
                        data['user'], str(e))
         finally:
             if conn.isconnected():
-                conn.destroy_connection()
+                conn.disconnect()
 
         # Report status and return.
         response_headers.append(('X-IPA-TokenSync-Result', result))
-- 
2.1.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