Skip to content

Potential memory leak/bug in the valkey client #1023

Open
@jskrzypek

Description

@jskrzypek

I think there might be an issue with the __aenter__ method on the ValkeyBackend.

async def __aenter__(self) -> Self:
self.client = await GlideClient.create(self.config)
return self
async def __aexit__(self, *args, **kwargs) -> None:
await self.client.close()

This code snippet explains my thinking.

async def cache_operations():
    client_1 = None
    async with ValkeyCache(
        config=GlideClientConfiguration(addresses=[NodeAddress("localhost", 6379)])
    ) as my_cache_1: # <<< new client created
        client_1 = my_cache_1.client
        async with my_cache_1 as my_cache_2: # <<< new client created
            assert my_cache_1 is my_cache_2  # because of return self
            client_2 = my_cache_2
            assert client_1 != client_2
            assert my_cache_1.client == client_2
            # exit inner with, call __aexit__, calls await self.client.close(); i.e. client_2.close())
        assert my_cache_1.client != client_1 # it's still client_2
        # exit inner with, call __aexit__, calls await self.client.close(); i.e. client_2.close() again, which may error.
    # client_1.close() may never be called

I think that the __aenter__ method should maybe handle checking if an existing connection is available and disposing of it if not, or else return a clone of self with a new client to ensure that the connection does get managed correctly.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions