Hacktoberfest 2026：メンテナが10月に向けて印を付けた、オープンで初心者向けの issue。 Hacktoberfest の issue を見る

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

クローズ
  • 何か問題がありますか？
#660 コメント 2 件 リアクション 0 件 担当者 0 名 GitHub で見る

メンテナーはふだん 5 日以内に返信

まだ誰も着手していません。

  • #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時間
マージ済み PR（30日）
16

環境構築

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

redis/redis-vl-python のほかの issue

redis/redis-vl-python の issue をすべて見る

似ている issue

Python の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。