Skip to content

Commit c8a6938

Browse files
committed
fix(backends/chroma): release SQLite file lock on close_palace/close (#1067)
ChromaBackend.close_palace() and close() evicted cached PersistentClients from self._clients without calling client.close(), so chromadb 1.5.x kept the rust-side SQLite file lock until GC. Reopening the same palace path after shutil.rmtree + re-create within one process then failed with SQLITE_READONLY_DBMOVED (SQLite code 1032). Add _close_client() helper with a try/except fallback for older chromadb, and route close_palace(), close(), and the DB-file-missing invalidation branch of _client() through it. The mtime/inode auto-invalidation branch is left as-is: callers there may still hold a live ChromaCollection handle, and closing out from under them clears the rust bindings mid-use. Regression tests cover close_palace reopen-same-path and whole-backend close for multiple palaces.
1 parent 9b35d9f commit c8a6938

2 files changed

Lines changed: 74 additions & 3 deletions

File tree

mempalace/backends/chroma.py

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,20 @@ def _as_list(v: Any) -> list:
177177
return [v]
178178

179179

180+
def _close_client(client) -> None:
181+
"""Call ``PersistentClient.close()`` if available, swallow otherwise.
182+
183+
chromadb 1.5.x exposes ``Client.close()`` to release rust-side SQLite
184+
file locks; older versions relied on GC. Try/except keeps forward-compat.
185+
"""
186+
if client is None:
187+
return
188+
try:
189+
client.close()
190+
except Exception:
191+
logger.debug("client.close() unavailable or failed", exc_info=True)
192+
193+
180194
class ChromaCollection(BaseCollection):
181195
"""Thin adapter translating ChromaDB dict returns into typed results."""
182196

@@ -449,7 +463,7 @@ def _client(self, palace_path: str):
449463
db_path = os.path.join(palace_path, "chroma.sqlite3")
450464
# DB was present when cache was built but is now missing → invalidate.
451465
if cached is not None and not os.path.isfile(db_path):
452-
self._clients.pop(palace_path, None)
466+
_close_client(self._clients.pop(palace_path, None))
453467
self._freshness.pop(palace_path, None)
454468
cached = None
455469
cached_inode, cached_mtime = 0, 0.0
@@ -542,14 +556,21 @@ def get_collection(
542556
return ChromaCollection(collection)
543557

544558
def close_palace(self, palace) -> None:
545-
"""Drop cached handles for ``palace``. Accepts ``PalaceRef`` or legacy path str."""
559+
"""Drop cached handles for ``palace``. Accepts ``PalaceRef`` or legacy path str.
560+
561+
Calls ``PersistentClient.close()`` before evicting so chromadb releases
562+
its SQLite file lock and the palace path can be safely reopened or
563+
removed in the same process (see #1067).
564+
"""
546565
path = palace.local_path if isinstance(palace, PalaceRef) else palace
547566
if path is None:
548567
return
549-
self._clients.pop(path, None)
568+
_close_client(self._clients.pop(path, None))
550569
self._freshness.pop(path, None)
551570

552571
def close(self) -> None:
572+
for client in self._clients.values():
573+
_close_client(client)
553574
self._clients.clear()
554575
self._freshness.clear()
555576
self._closed = True

tests/test_backends.py

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,56 @@ def test_query_empty_preserves_embeddings_outer_shape_when_requested():
204204
assert not_requested.embeddings is None
205205

206206

207+
def test_chroma_close_palace_releases_sqlite_lock_for_reopen(tmp_path):
208+
"""close_palace must release chromadb's SQLite file lock so a fresh
209+
PersistentClient on the same path (after shutil.rmtree + re-create)
210+
can write without hitting SQLITE_READONLY_DBMOVED (#1067)."""
211+
import shutil
212+
213+
backend = ChromaBackend()
214+
palace_path = tmp_path / "palace-a"
215+
ref = PalaceRef(id=str(palace_path), local_path=str(palace_path))
216+
217+
col = backend.get_collection(palace=ref, collection_name="mempalace_drawers", create=True)
218+
col.upsert(documents=["hello"], ids=["a"], metadatas=[{"k": "v"}])
219+
220+
backend.close_palace(ref)
221+
shutil.rmtree(palace_path)
222+
223+
col = backend.get_collection(palace=ref, collection_name="mempalace_drawers", create=True)
224+
col.upsert(documents=["world"], ids=["b"], metadatas=[{"k": "v2"}])
225+
assert col.count() == 1
226+
227+
228+
def test_chroma_close_releases_all_cached_clients(tmp_path):
229+
"""close() must release every cached client's SQLite file lock so any
230+
of their palace paths can be reopened by a fresh backend in the same
231+
process (#1067)."""
232+
import shutil
233+
234+
backend = ChromaBackend()
235+
palace_a = tmp_path / "palace-a"
236+
palace_b = tmp_path / "palace-b"
237+
ref_a = PalaceRef(id=str(palace_a), local_path=str(palace_a))
238+
ref_b = PalaceRef(id=str(palace_b), local_path=str(palace_b))
239+
240+
for ref in (ref_a, ref_b):
241+
backend.get_collection(palace=ref, collection_name="mempalace_drawers", create=True).upsert(
242+
documents=["x"], ids=["x"], metadatas=[{"k": "v"}]
243+
)
244+
245+
backend.close()
246+
247+
for path in (palace_a, palace_b):
248+
shutil.rmtree(path)
249+
ref = PalaceRef(id=str(path), local_path=str(path))
250+
col = ChromaBackend().get_collection(
251+
palace=ref, collection_name="mempalace_drawers", create=True
252+
)
253+
col.upsert(documents=["y"], ids=["y"], metadatas=[{"k": "v2"}])
254+
assert col.count() == 1
255+
256+
207257
def test_chroma_cache_invalidates_when_db_file_missing(tmp_path):
208258
"""A palace rebuild that removes chroma.sqlite3 must drop the stale cache.
209259

0 commit comments

Comments
 (0)