Skip to content

Commit 9585ea0

Browse files
committed
Fix concurrent access causing file lock conflicts in FFI bindings
Signed-off-by: Avelino <31996+avelino@users.noreply.github.com>
1 parent d2f745f commit 9585ea0

File tree

6 files changed

+1082
-82
lines changed

6 files changed

+1082
-82
lines changed

bindings/python/chrondb/client.py

Lines changed: 146 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
"""High-level Python client for ChronDB."""
22

33
import json
4+
import os
5+
import threading
46
from ctypes import c_void_p, pointer
5-
from typing import Any, Dict, List, Optional
7+
from typing import Any, Dict, List, Optional, Tuple
68

79
from chrondb._ffi import load_library
810

@@ -17,6 +19,124 @@ class DocumentNotFoundError(ChronDBError):
1719
pass
1820

1921

22+
# Global registry for shared isolates per path pair.
23+
# This ensures multiple ChronDB instances for the same paths share
24+
# the same GraalVM isolate, avoiding file lock conflicts.
25+
_isolate_registry: Dict[Tuple[str, str], "_SharedIsolate"] = {}
26+
_registry_lock = threading.Lock()
27+
28+
29+
class _SharedIsolate:
30+
"""Shared isolate that can be used by multiple ChronDB instances."""
31+
32+
def __init__(self, lib, data_path: str, index_path: str):
33+
self._lib = lib
34+
self.data_path = data_path
35+
self.index_path = index_path
36+
self._ref_count = 1
37+
self._lock = threading.Lock()
38+
39+
# Create isolate
40+
self._isolate = c_void_p()
41+
self._thread = c_void_p()
42+
43+
ret = lib.graal_create_isolate(
44+
None, pointer(self._isolate), pointer(self._thread)
45+
)
46+
if ret != 0:
47+
raise ChronDBError("Failed to create GraalVM isolate")
48+
49+
# Open database
50+
self._handle = lib.chrondb_open(
51+
self._thread,
52+
data_path.encode("utf-8"),
53+
index_path.encode("utf-8"),
54+
)
55+
if self._handle < 0:
56+
err = self._get_last_error()
57+
lib.graal_tear_down_isolate(self._thread)
58+
raise ChronDBError(f"Failed to open database: {err}")
59+
60+
def _get_last_error(self) -> str:
61+
"""Get the last error message from the native library."""
62+
err = self._lib.chrondb_last_error(self._thread)
63+
if err is None:
64+
return "unknown error"
65+
return err.decode("utf-8")
66+
67+
def add_ref(self):
68+
"""Increment reference count."""
69+
with self._lock:
70+
self._ref_count += 1
71+
72+
def release(self) -> bool:
73+
"""Decrement reference count. Returns True if isolate was closed."""
74+
with self._lock:
75+
self._ref_count -= 1
76+
if self._ref_count <= 0:
77+
self._close()
78+
return True
79+
return False
80+
81+
def _close(self):
82+
"""Close the isolate and release resources."""
83+
if self._handle >= 0:
84+
self._lib.chrondb_close(self._thread, self._handle)
85+
self._handle = -1
86+
if self._thread:
87+
self._lib.graal_tear_down_isolate(self._thread)
88+
self._thread = c_void_p()
89+
self._isolate = c_void_p()
90+
91+
@property
92+
def lib(self):
93+
return self._lib
94+
95+
@property
96+
def thread(self):
97+
return self._thread
98+
99+
@property
100+
def handle(self):
101+
return self._handle
102+
103+
104+
def _normalize_path(path: str) -> str:
105+
"""Normalize path for consistent registry keys."""
106+
try:
107+
return os.path.realpath(path)
108+
except OSError:
109+
return path
110+
111+
112+
def _get_or_create_isolate(data_path: str, index_path: str) -> _SharedIsolate:
113+
"""Get existing isolate for path pair, or create new one."""
114+
key = (_normalize_path(data_path), _normalize_path(index_path))
115+
116+
with _registry_lock:
117+
if key in _isolate_registry:
118+
isolate = _isolate_registry[key]
119+
isolate.add_ref()
120+
return isolate
121+
122+
# Create new isolate
123+
lib = load_library()
124+
isolate = _SharedIsolate(lib, data_path, index_path)
125+
_isolate_registry[key] = isolate
126+
return isolate
127+
128+
129+
def _release_isolate(data_path: str, index_path: str):
130+
"""Release reference to isolate. Closes if no more references."""
131+
key = (_normalize_path(data_path), _normalize_path(index_path))
132+
133+
with _registry_lock:
134+
if key in _isolate_registry:
135+
isolate = _isolate_registry[key]
136+
if isolate.release():
137+
del _isolate_registry[key]
138+
139+
20140
class ChronDB:
21141
"""A connection to a ChronDB database instance.
22142
@@ -25,6 +145,9 @@ class ChronDB:
25145
with ChronDB("/tmp/data", "/tmp/index") as db:
26146
db.put("user:1", {"name": "Alice"})
27147
doc = db.get("user:1")
148+
149+
Multiple instances opening the same paths share a single GraalVM isolate,
150+
ensuring thread-safe concurrent access without file lock conflicts.
28151
"""
29152

30153
def __init__(self, data_path: str, index_path: str):
@@ -37,25 +160,22 @@ def __init__(self, data_path: str, index_path: str):
37160
Raises:
38161
ChronDBError: If the database cannot be opened.
39162
"""
40-
self._lib = load_library()
41-
self._isolate = c_void_p()
42-
self._thread = c_void_p()
163+
self._data_path = data_path
164+
self._index_path = index_path
165+
self._shared = _get_or_create_isolate(data_path, index_path)
166+
self._closed = False
43167

44-
ret = self._lib.graal_create_isolate(
45-
None, pointer(self._isolate), pointer(self._thread)
46-
)
47-
if ret != 0:
48-
raise ChronDBError("Failed to create GraalVM isolate")
168+
@property
169+
def _lib(self):
170+
return self._shared.lib
49171

50-
self._handle = self._lib.chrondb_open(
51-
self._thread,
52-
data_path.encode("utf-8"),
53-
index_path.encode("utf-8"),
54-
)
55-
if self._handle < 0:
56-
err = self._get_last_error()
57-
self._lib.graal_tear_down_isolate(self._thread)
58-
raise ChronDBError(f"Failed to open database: {err}")
172+
@property
173+
def _thread(self):
174+
return self._shared.thread
175+
176+
@property
177+
def _handle(self):
178+
return self._shared.handle
59179

60180
def __enter__(self):
61181
return self
@@ -65,14 +185,14 @@ def __exit__(self, exc_type, exc_val, exc_tb):
65185
return False
66186

67187
def close(self):
68-
"""Close the database connection and release resources."""
69-
if self._handle >= 0:
70-
self._lib.chrondb_close(self._thread, self._handle)
71-
self._handle = -1
72-
if self._thread:
73-
self._lib.graal_tear_down_isolate(self._thread)
74-
self._thread = c_void_p()
75-
self._isolate = c_void_p()
188+
"""Close the database connection and release resources.
189+
190+
The underlying GraalVM isolate is only closed when all ChronDB
191+
instances using the same paths have been closed.
192+
"""
193+
if not self._closed:
194+
self._closed = True
195+
_release_isolate(self._data_path, self._index_path)
76196

77197
def put(self, id: str, doc: Dict[str, Any], branch: Optional[str] = None) -> Dict[str, Any]:
78198
"""Save a document.

0 commit comments

Comments
 (0)