bonsai
bonsai copied to clipboard
Why connection.close() can raise an exception?
I'm using AIOConnectionPool and try to check its behavior if LDAP server become unavailable.
Initial setup - I've started LDAP server on localhost, and try to search for a specific user:
Run local LDAP server
Using https://github.com/docker-ThoTeam/slapd-server-mock:
docker run --rm -v ./data.ldif.TEMPLATE:/bootstrap/data.ldif.TEMPLATE -p 389:389 thoteam/slapd-server-mock
>>> import logging
>>> from bonsai import LDAPClient, LDAPError, LDAPSearchScope
>>> from bonsai.asyncio import AIOConnectionPool
>>> log = logging.getLogger()
>>> client = LDAPClient("ldap://localhost")
>>> pool = AIOConnectionPool(client)
>>> async with pool.spawn() as connection:
>>> results = await connection.search(base="ou=people,dc=ldapmock,dc=local", filter_exp="uid=developer1", scope=LDAPSearchScope.SUBTREE)
>>> results
[{'dn': <LDAPDN uid=developer1,ou=people,dc=ldapmock,dc=local>, 'objectClass': ['inetOrgPerson', 'organizationalPerson', 'person', 'top'], 'cn': ['developer One'], 'sn': ['One'], 'givenName': ['developer'], 'mail': ['[email protected]'], 'uid': ['developer1']}]
>>> connection
<bonsai.asyncio.aioconnection.AIOLDAPConnection object at 0x7f922fbd3770>
>>> connection.closed
False
Pool now has an opened connection. Stop LDAP server, and try to search for user again
try:
async with pool.spawn() as connection:
try:
results = await connection.search(base="ou=people,dc=ldapmock,dc=local", filter_exp="uid=developer1", scope=LDAPSearchScope.SUBTREE)
except LDAPError:
log.exception("Exception during search")
connection.close()
except LDAPError:
log.error("Exception during spawn")
raise
Calling connection.close()
allows to skip returning connection back to pool, so next access to the same pool will initialize a new connection: https://github.com/noirello/bonsai/blob/74ce8f8548a04ab763a21ffaf4bef3ef529ced18/docs/advanced.rst#connection-timeouts
But connection.close()
raises an exception, so connection is not being closed:
Exception during search
Traceback (most recent call last):
File "<console>", line 4, in <module>
File "/home/maxim/Repo/horizon/.venv/lib/python3.11/site-packages/bonsai/asyncio/aioconnection.py", line 59, in _poll
raise exc
File "/home/maxim/Repo/horizon/.venv/lib/python3.11/site-packages/bonsai/asyncio/aioconnection.py", line 54, in _poll
return await asyncio.wait_for(fut, timeout)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/asyncio/tasks.py", line 452, in wait_for
return await fut
^^^^^^^^^
File "/home/maxim/Repo/horizon/.venv/lib/python3.11/site-packages/bonsai/asyncio/aioconnection.py", line 40, in _ready
res = super().get_result(msg_id)
^^^^^^^^^^^^^^^^^^^^^^^^^^
bonsai.errors.ConnectionError: Can't contact LDAP server. (0xFFFF [-1])
Exception during spawn
Traceback (most recent call last):
File "<console>", line 4, in <module>
File "/home/maxim/Repo/horizon/.venv/lib/python3.11/site-packages/bonsai/asyncio/aioconnection.py", line 59, in _poll
raise exc
File "/home/maxim/Repo/horizon/.venv/lib/python3.11/site-packages/bonsai/asyncio/aioconnection.py", line 54, in _poll
return await asyncio.wait_for(fut, timeout)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/asyncio/tasks.py", line 452, in wait_for
return await fut
^^^^^^^^^
File "/home/maxim/Repo/horizon/.venv/lib/python3.11/site-packages/bonsai/asyncio/aioconnection.py", line 40, in _ready
res = super().get_result(msg_id)
^^^^^^^^^^^^^^^^^^^^^^^^^^
bonsai.errors.ConnectionError: Can't contact LDAP server. (0xFFFF [-1])
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/usr/lib/python3.11/concurrent/futures/_base.py", line 456, in result
return self.__get_result()
^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/concurrent/futures/_base.py", line 401, in __get_result
raise self._exception
File "<console>", line 7, in <module>
bonsai.errors.ConnectionError: Can't contact LDAP server. (0xFFFF [-1])
>>> connection
<bonsai.asyncio.aioconnection.AIOLDAPConnection object at 0x7f922fbd3770>
>>> connection.closed
False
Because connection.close()
raised an exception, it looks like it haven't changed connection.closed
flag to True
. This connection is broken now, it cannot be used for making any requests.
For example, start LDAP server again and use the same pool. Because connection.closed
is not True
, it is still in the pool and being reused:
try:
async with pool.spawn() as connection:
try:
results = await connection.search(base="ou=people,dc=ldapmock,dc=local", filter_exp="uid=developer1", scope=LDAPSearchScope.SUBTREE)
except LDAPError:
log.exception("Exception during search")
connection.close()
except LDAPError:
log.error("Exception during spawn")
raise
Exception during search
Traceback (most recent call last):
File "<console>", line 4, in <module>
File "/home/maxim/Repo/horizon/.venv/lib/python3.11/site-packages/bonsai/asyncio/aioconnection.py", line 59, in _poll
raise exc
File "/home/maxim/Repo/horizon/.venv/lib/python3.11/site-packages/bonsai/asyncio/aioconnection.py", line 54, in _poll
return await asyncio.wait_for(fut, timeout)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/asyncio/tasks.py", line 452, in wait_for
return await fut
^^^^^^^^^
File "/home/maxim/Repo/horizon/.venv/lib/python3.11/site-packages/bonsai/asyncio/aioconnection.py", line 40, in _ready
res = super().get_result(msg_id)
^^^^^^^^^^^^^^^^^^^^^^^^^^
bonsai.errors.LDAPError: Success. (0x0000 [0])
Well, something is definitely went wrong. Fortunately, connection now become closed, so next requests made using the same pool working as expected:
>>> connection
<bonsai.asyncio.aioconnection.AIOLDAPConnection object at 0x7f922fbd3770>
>>> connection.closed
True
>>> try:
... async with pool.spawn() as connection:
... try:
... results = await connection.search(base="ou=people,dc=ldapmock,dc=local", filter_exp="uid=developer1", scope=LDAPSearchScope.SUBTREE)
... except LDAPError:
.. log.exception("Exception during search")
... connection.close()
... except LDAPError:
... log.error("Exception during spawn")
... raise
>>> results
[{'dn': <LDAPDN uid=developer1,ou=people,dc=ldapmock,dc=local>, 'objectClass': ['inetOrgPerson', 'organizationalPerson', 'person', 'top'], 'cn': ['developer One'], 'sn': ['One'], 'givenName': ['developer'], 'mail': ['[email protected]'], 'uid': ['developer1']}]
>>> connection
<bonsai.asyncio.aioconnection.AIOLDAPConnection object at 0x7f922effc670>
>>> connection.closed
False
Few more things bother me:
-
connection.close()
is synchronous in both LDAPConnection and AIOLDAPConnection, but it looks like it try to send network request to LDAP server for properly unbind/disconnect instead of just dropping connection. Current implementation ofAIOConnectionPool.close()
is asynchronous, and it calls this synchronous method, so it can block event loop: https://github.com/noirello/bonsai/blob/74ce8f8548a04ab763a21ffaf4bef3ef529ced18/src/bonsai/asyncio/aiopool.py#L82 https://github.com/noirello/bonsai/blob/74ce8f8548a04ab763a21ffaf4bef3ef529ced18/src/bonsai/pool.py#L115 -
Current
Pool.close()
implementation callsconn.close()
for all connections using a cycle. So if some connection raises an exception during close, other connections in the pool may left unclosed. The only workaround I could find is:
for _ in range(pool.shared_connections):
with contextlib.suppress(LDAPError):
pool.close()
Which does not look right.
Proposed solution:
- Change
connection.close()
implementation to never raise any exceptions, if that even possible. - If
AIOLDAPConnection.close()
requires some request to be made to LDAP server (unbind, explicit connection close), make itasync
, and useawait
inAIOConnectionPool.close()
- If establishing the connection is failed with any
LDAPError
(not justConnectionError
, but alsoAuthenticationError
,InvalidDN
,TimeoutError
and so on), immediately mark connection as closed without requiring to callconnection.close()
explicitly. This connection cannot be used anymore. - Change
ConnectionPool.close()
implementation to pop connection from set, e.g.:
def close(self) -> None:
while self._idles:
with suppress(Exception):
conn = self._idles.pop()
conn.close()
while self._used:
with suppress(Exception):
conn = self._used.pop()
conn.close()
self._closed = True
To avoid partial cleanup of the pool if some connection cannot be closed.