Hacktoberfest 2026：维护者为十月标记出来的 issue，仍然开放、适合新手。 浏览 Hacktoberfest issue

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

已关闭
  • 哪里不对？
#660 2 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看

维护者通常 5 天内回复

还没有人认领这个 Issue。

  • #661 来自 @nkanu17 —— 已关闭，未合并
  • #705 来自 @mikemikimike —— 已关闭，未合并

评估

难度
3/5
预计耗时
1-2 天
新手友好度
70/100
Issue 类型
缺陷
描述清晰度
描述清楚
活跃度
活跃
技术栈
python, redis
领域
backend, databases

调研方向

从 set_client()、connect()、disconnect() 和构造函数注入中 SearchIndex 与 AsyncSearchIndex 的所有权处理开始。追踪同步和异步客户端如何被最终处理或关闭，然后为构造函数注入、set_client() 和 connect() 添加覆盖，以验证调用方提供的客户端保持打开状态，而内部创建的客户端会被释放。

由索引模型根据 Issue 内容生成。

描述

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).

主要语言
Python
星标
431
派生
107
平均合并
5 天 17 小时
30 天内合并 PR
16

环境准备

  • 没有 Dockerfile 或 Docker Compose 文件
  • 没有 Pull Request 模板
  • 阅读贡献指南

从这里开始

  1. 先读完整个 Issue，再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库，在一个分支上完成修改。
  4. 提交 Pull Request，并在描述里引用这个 Issue 编号。

redis/redis-vl-python 的其他 Issue

查看 redis/redis-vl-python 的全部 Issue

相似的 Issue

更多 Python Issue

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。