Redis.from_pool(...) is not thread safe

[kain88-de]

We are using redis as a cache in a FastAPI app. To not make new connections to redis all the time we are using a Connection pool and initiate it startup. For each request we then initiate a Redis client from the pool, like so.

def redis_server(request: fapi.Request) -> redis.Redis:
    # https://www.starlette.io/lifespan/#lifespan-state
    # This is of type starlette.datastructures.state
    pool = typing.cast("redis.ConnectionPool", request.state.redis_pool)
    return redis.Redis.from_pool(connection_pool=pool)  # type: ignore[no-any-return]

This causes all connections to close when we end the request and drop the reference to the client object. With this each new request essentially creates a new connection, so the this is similar to no connection pool.

Now FastAPI runs "sync" requests in a thread pool. So when we have two simultaneous requests, both create a connection, but if just one finishes both connections are closed.

We noticed this with fun exceptions like this.

 "exc_type": "ValueError",
      "exc_value": "I/O operation on closed file.",

Where the buffer of a connection was was randomly closed in the middle of a healthcheck.

Now I know that from_pool documents it consumes the connection pool. But that concept is not really possible in python. As long as the reference to the connection pool lives the connection pool will live. There is nothing stopping us from passing it to different threads, even indirectly as here.

I understand the from_pool function was introduced to saving a pool.close() call after you are finished with the client object. I can understand that but a context manager on the connection pool sounds like a better way to achieve the same without any of the pitfalls.

with ConnectionPool(...) as pool:
  redis = Redis(connection_pool=pool)