-
Notifications
You must be signed in to change notification settings - Fork 290
fix(local): release .lock file via atexit hook (#765) #1365
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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) | ||
|
Comment on lines
+203
to
+205
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
git ls-files 'qdrant_client/local/qdrant_local.py' 'qdrant_client/local/async_qdrant_local.py'
printf '%s\n' '--- synchronous implementation ---'
sed -n '105,220p' qdrant_client/local/qdrant_local.py
printf '%s\n' '--- asynchronous implementation ---'
sed -n '105,205p' qdrant_client/local/async_qdrant_local.py
printf '%s\n' '--- lifecycle and atexit references ---'
rg -n -C 3 'atexit|def close|_release_lock|portalocker\.(lock|unlock)' qdrant_client/local/qdrant_local.py qdrant_client/local/async_qdrant_local.pyRepository: qdrant/qdrant-client Length of output: 19488 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- lifecycle methods and constructor calls ---'
sed -n '45,125p' qdrant_client/local/qdrant_local.py
sed -n '45,125p' qdrant_client/local/async_qdrant_local.py
printf '%s\n' '--- cleanup and callback removal across the repository ---'
rg -n -C 3 'atexit\.unregister|atexit\.register|\.close\(\)' qdrant_client tests 2>/dev/null | head -n 240 || true
printf '%s\n' '--- standalone bound-method retention probe ---'
python3 - <<'PY'
import atexit
import gc
import weakref
class Backend:
def __init__(self):
self.collections = {"large": bytearray(1024)}
self.closed = False
def close(self):
self.closed = True
def release(self):
pass
backend = Backend()
reference = weakref.ref(backend)
callback = backend.release
atexit.register(callback)
backend.close()
del callback
del backend
gc.collect()
print("retained_after_close:", reference() is not None)
atexit.unregister(reference().release)
del reference
gc.collect()
print("callback_removed:", "completed")
PYRepository: qdrant/qdrant-client Length of output: 22329 Unregister the
📍 Affects 2 files
🤖 Prompt for AI Agents |
||
|
|
||
| def _save(self) -> None: | ||
| if not self.persistent: | ||
| return | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: qdrant/qdrant-client
Length of output: 44996
🏁 Script executed:
Repository: qdrant/qdrant-client
Length of output: 266
Always close the lock file after an unlock failure.
If
portalocker.unlock()raises,self._flock_file.close()is skipped. Use afinallyblock with separate error handling for the close operation in both implementations.🧰 Tools
🪛 Ruff (0.16.1)
[error] 137-141:
try-except-passdetected, consider logging the exception(S110)
[warning] 137-137: Do not catch blind exception:
Exception(BLE001)
📍 Affects 2 files
qdrant_client/local/qdrant_local.py#L133-L141(this comment)qdrant_client/local/async_qdrant_local.py#L131-L139🤖 Prompt for AI Agents