noirello / bonsai

Simple Python 3 module for LDAP, using libldap2 and winldap C libraries.
MIT License
116 stars 32 forks source link

AIOLDAPConnection has loop affinity #69

Open gstein opened 1 year ago

gstein commented 1 year ago

I've been working on code to solve #25 and #41 by moving the .connect() into a separate thread via the Executor capabilities of asyncio. I've got this working, but have found the connection object holds onto the loop it was constructed with.

What this means, is that I cannot construct the connection on a background thread/loop, and then use it on the main thread/loop going forwards. All future operations must go back to that original loop (running on a distinct thread, within an Executor).

To remedy this, I've set up a system where any operation request goes through the Executor, and gets passed over to the original thread and loop assigned to that thread. Not fun, but the main loop is never bothered while this stuff is threshing around. My test code shows a regular heartbeat on the primary event loop.

I really do not have a suggestion. This is some deep magic in the AIOLDAPConnection class, caused by its needs to manage file descriptors on the loop. I don't see an obvious fix; it feels like a deeper design decision. Sigh.

gstein commented 1 year ago

For the record, here is the code that I use to establish the connection.

    def connect(self, loop=None):
        if loop is None:
            loop = asyncio.get_running_loop()

        # Hack around GnuTLS bug with async.
        # See: https://github.com/noirello/bonsai/issues/25
        #
        # In an executor thread, we'll perform the synchronous connection,
        # so that the event loop will not be blocked.
        #
        # TLOOP was created when the thread started, and holds
        # the event loop for all operations on this thread.
        def blocking_connect():
            tloop = self.loops[threading.current_thread()]
            async def do_connect():
                # Tell bonsai to connect synchronously.
                bonsai.set_connect_async(False)
                try:
                    return await self.client.connect(is_async=True)
                finally:
                    # Make sure this always gets reset.
                    bonsai.set_connect_async(True)

            conn = tloop.run_until_complete(do_connect())
            return self.CONNECTION_CLASS(self.executor, tloop, conn)

        future = loop.run_in_executor(self.executor, blocking_connect)
        return ConnectContextManager(future)
gstein commented 1 year ago

[ EDIT: this does not work. See next comment. -gstein ]

And how to actually use the connection, retaining its async properties. I've observed two connections using the same thread/loop, though the Executor will spin up a new thread if congestion occurs.

    async def search(self, base, attrs, loop=None):
        if loop is None:
            loop = asyncio.get_running_loop()

        def run_search():
            # This synchronous function is now running within a thread
            # of the Executor. Use some async to run the LDAP search.
            return self.loop.run_until_complete(
                self.conn.search(base,
                                 bonsai.LDAPSearchScope.SUBTREE,
                                 attrlist=attrs))

        # Wait within the caller's loop for the result.
        return await loop.run_in_executor(self.executor, run_search)
gstein commented 1 year ago

Actually, ignore that last comment. It does not properly steer the invocation back to the same thread/loop. The Executor may end up choosing a different thread, and things will break. Hard. ... max_workers=1 on the Executor will fix, but ugh.

gstein commented 1 year ago

My new/revised code pairs a bonsai LDAPConnection with an event loop. These loops can be used on any thread within the Executor. This ensures that an LDAPConnection continues to operate with its associated loop.