Hacktoberfest 2026: los issues que los mantenedores marcaron para octubre, abiertos y aptos para principiantes. Explorar issues de Hacktoberfest

set_client() keeps client ownership, so the index closes a caller-provided client

Cerrado
  • ¿Algo no está bien?
#660 2 comentarios 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 5 días

Nadie ha tomado este issue todavía.

  • #661 de @nkanu17 — cerrado sin fusionar
  • #705 de @mikemikimike — cerrado sin fusionar

Evaluación

Dificultad
3/5
Tiempo estimado
1-2 días
Aptitud para principiantes
70/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Activo
Stack tecnológico
python, redis
Área
backend, databases

Línea de trabajo

Comienza con la gestión de la propiedad de SearchIndex y AsyncSearchIndex en set_client(), connect(), disconnect() y la inyección en el constructor. Rastrea cómo se finalizan o cierran los clientes síncronos y asíncronos, y después añade cobertura para la inyección en el constructor, set_client() y connect() para verificar que los clientes proporcionados por quien realiza la llamada permanecen abiertos, mientras que los clientes creados internamente se liberan.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

Summary

An index created with redis_url owns the client it lazily creates, so _owns_redis_client is True. The deprecated set_client() does not reset that flag, so after a caller swaps in their own client the index still believes it owns it and will close it. __init__(redis_client=...) already gets this right and marks such a client as not owned.

Before 0.25.0 this was latent, because the client finalizer never fired (see #657). Now that the finalizer works, the index closes the caller's client when it is garbage collected.

Affects both SearchIndex and AsyncSearchIndex, on 0.25.0.

Reproduction

import gc
from unittest import mock
from redisvl.index import SearchIndex

SCHEMA = {
    "index": {"name": "probe", "prefix": "p", "storage_type": "hash"},
    "fields": [{"name": "a", "type": "tag"}],
}

caller_client = mock.MagicMock()
index = SearchIndex.from_dict(SCHEMA, redis_url="redis://localhost:6379")
with mock.patch("redisvl.index.index.RedisConnectionFactory.validate_sync_redis"):
    index.set_client(caller_client)

print("owns:", index._owns_redis_client)      # True, should be False
del index
gc.collect()
print("caller client closed:", caller_client.close.called)   # True, should be False

Observed on 0.25.0:

owns: True
caller client closed: True

The async path behaves the same, awaiting aclose() on the caller's client. Calling disconnect() explicitly closes it too, for the same reason.

Impact

Low severity but real. redis-py clients recover from close() and aclose() by reconnecting on next use, so the practical effect is unexpected connection churn on a client the caller still owns rather than a permanently broken client. Verified: ping() returns True after both close() and aclose() for sync and async clients.

The exposure is further limited because set_client() is deprecated.

Additional problem in the same method

The sync set_client() also abandons the client the index created for itself, without closing it. It overwrites __redis_client and (since 0.25.0) detaches that client's finalizer, so nothing ever closes it. The async set_client() does not have this problem because it awaits disconnect() before swapping.

Suggested fix

set_client() should mark the client as not owned, since a caller-provided client is by definition not the index's to close, and it should release the previously owned client first.

The deprecated async connect() needs care here: it creates its own client and then delegates to set_client(), so a naive ownership flip would leave a client the index created with nobody to close it. That path must keep ownership. Routing both through a small internal helper that takes ownership as a parameter handles this.

Worth covering with tests on all three entry points: constructor injection (already correct), set_client() (must not own), and connect() (must own).

Lenguaje dominante
Python
Estrellas
432
Forks
111
Merge medio
5 d 17 h
PR fusionados (30 d)
16

Preparar el entorno

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de redis/redis-vl-python

Todos los issues de redis/redis-vl-python

Issues similares

Más issues de Python

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.