diff --git a/qdrant_client/local/async_qdrant_local.py b/qdrant_client/local/async_qdrant_local.py index 971ad5e87..8f0e0b0e8 100644 --- a/qdrant_client/local/async_qdrant_local.py +++ b/qdrant_client/local/async_qdrant_local.py @@ -9,6 +9,7 @@ # # ****** WARNING: THIS FILE IS AUTOGENERATED ****** +import atexit import importlib.metadata import itertools import json @@ -112,6 +113,31 @@ async def close(self, **kwargs: Any) -> None: except TypeError: pass + def _release_lock(self) -> None: + """Release the `.lock` file's OS-level lock and close its file handle. + + Registered as an `atexit` hook in `_load()` so the lock is released + even when the script exits without an explicit `.close()` call - + gradio hot-reloads, one-shot scripts, a worker that the supervisor + tears down without running finalizers. Without the hook, the OS + lock is only released when the OS reaps the file handle, which on + gradio reloads surfaces as a spurious "already accessed" + `RuntimeError` on the next process. The hook is idempotent: it + early-returns when the handle is already closed, so it composes + safely with an explicit `close()`. + """ + if self._flock_file is None or self._flock_file.closed: + return + try: + import portalocker # same import-deferral rationale as `close()` + portalocker.unlock(self._flock_file) + self._flock_file.close() + except (TypeError, Exception): + # Same teardown-safety rationale as `close()`: portalocker can + # be GC'd before the instance, and any other shutdown error + # must not crash interpreter teardown. + pass + def _load(self) -> None: deprecated_config_fields = ("init_from",) if not self.persistent: @@ -160,6 +186,10 @@ def _load(self) -> None: f"Storage folder {self.location} is already accessed by another instance of Qdrant client. If you require concurrent access, use Qdrant server instead." ) + # Register the atexit hook only after the lock is held, so a + # process that loses the race (above) never gets registered. + atexit.register(self._release_lock) + def _save(self) -> None: if not self.persistent: return diff --git a/qdrant_client/local/qdrant_local.py b/qdrant_client/local/qdrant_local.py index 9e02df825..8fbf9b01d 100644 --- a/qdrant_client/local/qdrant_local.py +++ b/qdrant_client/local/qdrant_local.py @@ -1,3 +1,4 @@ +import atexit import importlib.metadata import itertools import json @@ -114,6 +115,31 @@ def close(self, **kwargs: Any) -> None: # QdrantLocal instance pass + def _release_lock(self) -> None: + """Release the `.lock` file's OS-level lock and close its file handle. + + Registered as an `atexit` hook in `_load()` so the lock is released + even when the script exits without an explicit `.close()` call - + gradio hot-reloads, one-shot scripts, a worker that the supervisor + tears down without running finalizers. Without the hook, the OS + lock is only released when the OS reaps the file handle, which on + gradio reloads surfaces as a spurious "already accessed" + `RuntimeError` on the next process. The hook is idempotent: it + early-returns when the handle is already closed, so it composes + safely with an explicit `close()`. + """ + if self._flock_file is None or self._flock_file.closed: + return + try: + import portalocker # same import-deferral rationale as `close()` + portalocker.unlock(self._flock_file) + self._flock_file.close() + except (TypeError, Exception): + # Same teardown-safety rationale as `close()`: portalocker can + # be GC'd before the instance, and any other shutdown error + # must not crash interpreter teardown. + pass + def _load(self) -> None: deprecated_config_fields = ("init_from",) @@ -174,6 +200,10 @@ def _load(self) -> None: f" If you require concurrent access, use Qdrant server instead." ) + # Register the atexit hook only after the lock is held, so a + # process that loses the race (above) never gets registered. + atexit.register(self._release_lock) + def _save(self) -> None: if not self.persistent: return diff --git a/tests/test_local_persistence.py b/tests/test_local_persistence.py index f095c0f13..673cf1c1d 100644 --- a/tests/test_local_persistence.py +++ b/tests/test_local_persistence.py @@ -201,3 +201,45 @@ def test_update_persistence(): "not_important": "missing", } client.close() + + +def test_lockfile_released_on_atexit_hook(): + """Regression test for https://github.com/qdrant/qdrant-client/issues/765. + + QdrantLocal registers an atexit hook that releases the `.lock` file's + OS-level lock. Without the hook, a process that exits without calling + `.close()` (gradio hot-reload, a one-shot script) leaves the OS lock + held until the OS reaps the file handle, and the next process that + opens the same path sees a spurious "already accessed" RuntimeError. + + We can't drive the real atexit path in-process (it only fires on + interpreter shutdown), so we test the helper directly: call + `_release_lock` to simulate the hook running, then verify a second + client can acquire the same path. + """ + with tempfile.TemporaryDirectory() as tmpdir: + first = QdrantClient(path=tmpdir) + + flock = first._client._flock_file + assert flock is not None + assert not flock.closed, "lockfile should be held while client is alive" + + first._client._release_lock() + assert flock.closed, "_release_lock should close the lockfile handle" + + second = QdrantClient(path=tmpdir) + second.close() + first.close() + + +def test_lockfile_release_lock_is_idempotent(): + """`_release_lock` must be a no-op when the handle is already closed, + so it composes safely with an explicit `.close()` followed by the + atexit hook firing at interpreter shutdown. + """ + with tempfile.TemporaryDirectory() as tmpdir: + client = QdrantClient(path=tmpdir) + client.close() + # Explicit close already released the lock. The atexit hook will + # still fire on shutdown; it must not raise. + client._client._release_lock()