From 8d5e95b94f8bfa6cb22a034327860e66bcb33e89 Mon Sep 17 00:00:00 2001 From: Bret Curtis Date: Tue, 25 Aug 2026 15:50:01 +0200 Subject: [PATCH] LDAPClient.unbind returns a Deferred that fires on actual disconnect (fixes #225) Also add LDAPClient.notifyOnDisconnect() so callers can register their own disconnect Deferreds without initiating an unbind. Matters for TLS teardown, which can take noticeably longer than TCP FIN and previously left callers hoping the loseConnection had actually completed. Backwards compatible: existing callers that ignore unbind()'s return value keep working; the ones that need to wait now can. --- docs/source/NEWS.rst | 5 +++++ ldaptor/protocols/ldap/ldapclient.py | 26 ++++++++++++++++++++++++++ ldaptor/test/test_ldapclient.py | 28 ++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+) diff --git a/docs/source/NEWS.rst b/docs/source/NEWS.rst index 27ad6923..7e826c55 100644 --- a/docs/source/NEWS.rst +++ b/docs/source/NEWS.rst @@ -82,6 +82,11 @@ Fixes in-memory back end grew a ``deleteOldRDN`` kwarg on ``move`` and the client-side ``ldapsyntax.LDAPEntry.move`` grew the same flag so callers can request either behaviour (#89). +- ``LDAPClient.unbind`` now returns a ``Deferred`` that fires with the + connection-lost reason once the transport has actually been closed, + which matters for TLS teardown. A new + ``LDAPClient.notifyOnDisconnect()`` helper lets callers register + additional disconnect Deferreds without initiating an unbind (#225). 21.2.0 (2021-02-28) diff --git a/ldaptor/protocols/ldap/ldapclient.py b/ldaptor/protocols/ldap/ldapclient.py index 7ebc4700..f2590f6f 100644 --- a/ldaptor/protocols/ldap/ldapclient.py +++ b/ldaptor/protocols/ldap/ldapclient.py @@ -39,6 +39,7 @@ def __init__(self): self.onwire = {} self.buffer = b"" self.connected = None + self._disconnect_deferreds = [] berdecoder = pureldap.LDAPBERDecoderContext_TopLevel( inherit=pureldap.LDAPBERDecoderContext_LDAPMessage( @@ -75,6 +76,21 @@ def connectionLost(self, reason=protocol.connectionDone): k, v = self.onwire.popitem() d, _, _, _, _ = v d.errback(reason) + # fire any notifyOnDisconnect / unbind Deferreds + pending = self._disconnect_deferreds + self._disconnect_deferreds = [] + for d in pending: + d.callback(reason) + + def notifyOnDisconnect(self): + """ + Return a Deferred that fires with the disconnect reason once the + transport has actually been closed. Useful for waiting on a clean + TLS shutdown after unbind() (#225). + """ + d = defer.Deferred() + self._disconnect_deferreds.append(d) + return d def _send(self, op, controls=None): if not self.connected: @@ -235,11 +251,21 @@ def _handle_bind_msg(self, msg): return (msg.matchedDN, msg.serverSaslCreds) def unbind(self): + """ + Send an LDAP unbind and close the connection. + + Returns a Deferred that fires with the connection-lost reason + once the transport has actually closed. Callers that don't need + to wait can ignore the return value; callers that do (e.g. + cleanup after a TLS shutdown) can chain on it. + """ if not self.connected: raise Exception("Not connected (TODO)") # TODO make this a real object r = pureldap.LDAPUnbindRequest() self.send_noResponse(r) + d = self.notifyOnDisconnect() self.transport.loseConnection() + return d def _cbStartTLS(self, msg, ctx): assert isinstance(msg, pureldap.LDAPExtendedResponse) diff --git a/ldaptor/test/test_ldapclient.py b/ldaptor/test/test_ldapclient.py index 874ef12f..d366bb27 100644 --- a/ldaptor/test/test_ldapclient.py +++ b/ldaptor/test/test_ldapclient.py @@ -130,6 +130,34 @@ def test_unbind_not_connected(self): client = ldapclient.LDAPClient() self.assertRaises(Exception, client.unbind) + def test_unbind_returnsDeferredFiringOnDisconnect(self): + """ + unbind() returns a Deferred that fires once the transport is + actually closed. Previously the caller had no way to await the + (potentially slow, e.g. TLS) shutdown (#225). + """ + client, transport = self.create_test_client() + d = client.unbind() + fired = [] + d.addBoth(fired.append) + self.assertEqual(fired, [], "unbind Deferred fired before disconnect") + client.connectionLost(SillyError("boom")) + self.assertEqual(len(fired), 1) + + def test_notifyOnDisconnect_firesEachTime(self): + """ + Multiple notifyOnDisconnect() Deferreds all fire on the same + connectionLost event. + """ + client, transport = self.create_test_client() + d1 = client.notifyOnDisconnect() + d2 = client.notifyOnDisconnect() + seen = [] + d1.addBoth(lambda r: seen.append(("d1", r))) + d2.addBoth(lambda r: seen.append(("d2", r))) + client.connectionLost(SillyError("bye")) + self.assertEqual(len(seen), 2) + def test_TLS_failure(self): clock = Clock() ldapclient.reactor = clock