From 115fc755d1fcc9c7b9f8517a1bc642d5d266a754 Mon Sep 17 00:00:00 2001 From: Philipp Hahn Date: Fri, 13 Jan 2023 14:08:40 +0100 Subject: [PATCH] fix(ReconnectLDAPObject): reconnect race condition There is a race condition in ReconnectLDAPObject._apply_method_s() when multiple threads use a shared ReconnectLDAPObject instance: > Traceback (most recent call last): ... > File "ldap/ldapobject.py", line 1259, in search_ext_s > return self._apply_method_s(SimpleLDAPObject.search_ext_s,*args,**kwargs) > File "ldap/ldapobject.py", line 1201, in _apply_method_s > self.reconnect(self._uri,retry_max=self._retry_max,retry_delay=self._retry_delay) > File "ldap/ldapobject.py", line 1181, in reconnect > SimpleLDAPObject.unbind_s(self) > File "ldap/ldapobject.py", line 905, in unbind_s > return self.unbind_ext_s(None,None) > File "ldap/ldapobject.py", line 889, in unbind_ext_s > msgid = self.unbind_ext(serverctrls,clientctrls) > File "ldap/ldapobject.py", line 881, in unbind_ext > res = self._ldap_call(self._l.unbind_ext,RequestControlTuples(serverctrls),RequestControlTuples(clientctrls)) > File "ldap/ldapobject.py", line 352, in __getattr__ > self.__class__.__name__,repr(name) > AttributeError: ReconnectLDAPObject has no attribute '_l' While the reconnect() is already protected by `self._reconnect_lock`, there is a small windows between the `unbind_s()` and `reconnect()` call, where CPython can switch threads: While the 1st threads is already waiting inside `reconnect()` for its next try, the 2nd threads will call `unbind_s()` outside that lock which will race with the 1st thread also calling `unbind_s()`: Whoever comes first will do the `del self._l` first, so the later threads will raise the above AttributeError. Move calling `unbind_s()` inside the locked region so that `self._l` is handled atomically. Add a new parameter `force` to either forcefully close any previous connection or keep re-use the previous connection if it still is supposed to work. --- Lib/ldap/ldapobject.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/Lib/ldap/ldapobject.py b/Lib/ldap/ldapobject.py index 9442e39b..7a9c17f6 100644 --- a/Lib/ldap/ldapobject.py +++ b/Lib/ldap/ldapobject.py @@ -893,7 +893,7 @@ def __setstate__(self,d): self._reconnect_lock = ldap.LDAPLock(desc='reconnect lock within %s' % (repr(self))) # XXX cannot pickle file, use default trace file self._trace_file = ldap._trace_file - self.reconnect(self._uri) + self.reconnect(self._uri,force=True) def _store_last_bind(self,_method,*args,**kwargs): self._last_bind = (_method,args,kwargs) @@ -914,11 +914,16 @@ def _restore_options(self): def passwd_s(self,*args,**kwargs): return self._apply_method_s(SimpleLDAPObject.passwd_s,*args,**kwargs) - def reconnect(self,uri,retry_max=1,retry_delay=60.0): + def reconnect(self,uri,retry_max=1,retry_delay=60.0,force=True): # Drop and clean up old connection completely # Reconnect self._reconnect_lock.acquire() try: + if hasattr(self,'_l'): + if force: + SimpleLDAPObject.unbind_s(self) + else: + return reconnect_counter = retry_max while reconnect_counter: counter_text = '%d. (of %d)' % (retry_max-reconnect_counter+1,retry_max) @@ -962,14 +967,12 @@ def reconnect(self,uri,retry_max=1,retry_delay=60.0): return # reconnect() def _apply_method_s(self,func,*args,**kwargs): - if not hasattr(self,'_l'): - self.reconnect(self._uri,retry_max=self._retry_max,retry_delay=self._retry_delay) + self.reconnect(self._uri,retry_max=self._retry_max,retry_delay=self._retry_delay,force=False) try: return func(self,*args,**kwargs) except ldap.SERVER_DOWN: - SimpleLDAPObject.unbind_s(self) # Try to reconnect - self.reconnect(self._uri,retry_max=self._retry_max,retry_delay=self._retry_delay) + self.reconnect(self._uri,retry_max=self._retry_max,retry_delay=self._retry_delay,force=True) # Re-try last operation return func(self,*args,**kwargs)