Skip to content

Commit e44dd47

Browse files
authored
fix(server): keep a failed cursor key write from bricking the data location (#2061)
* fix(server): keep a failed cursor key write from bricking the data location os.write may accept fewer bytes than requested. The previous code raised on a short write but left the incomplete key file behind, and _read() then rejects that file on every later start, so one failed write made the data location permanently unusable with no way to regenerate the key. Resume the write until every byte is on disk, and remove the key file when the write cannot complete so the next start can create a new one. A write that makes no progress is rejected instead of retried. * fix(server): recover a cursor key left by an interrupted creation A process killed between os.open and os.write leaves a key of the wrong length on disk, and _read() raised ValueError for it. resolve_cursor_secret runs from the server lifespan without a handler, so one crash made the data location unstartable on every later start. Treat a key of the wrong length as "no key yet": discard it and create a new one. That is safe because the interrupted writer never returned the key to a caller, so no cursor was signed with it. fsync the key before returning it so a successful write also survives a power loss. * fix(server): serialize cursor key recovery across starters A starter that read a missing key could wake up after another starter had published a complete one, delete it, and create a replacement. Both calls succeeded with different keys, so cursors signed by the second starter no longer verified against the key on disk. Recovery (decide a key is debris, delete it, create a replacement) now shares one cross-process lock with creation. A wrong-length file is only treated as debris while holding that lock, where no other starter can be midway through writing the key it is about to publish. The lock file is kept after release: removing it would let a waiter on the old inode and a newcomer on its replacement enter at the same time. * fix(server): lock the cursor key file without seeding a marker byte Two starters can both read the lock file as empty. If the first one pauses after the size check and the second one writes its marker and locks byte 0, the first one's write lands in a byte the second one has locked. Windows rejects that write with errno 13, and the error escapes the retry loop, so the first starter fails to start even though the persisted key is valid. `_locking` accepts a range past the end of the file, so the marker is not needed: locking byte 0 by itself keeps starters apart. The seeding step is gone, and no write happens before the lock is held. Checked with two real processes on Windows: a write into the held byte fails with errno 13 while a second lock attempt is refused, and a zero-length file locks and unlocks normally. The regression test enters the lock while another process holds byte 0 and asserts the wait, so a lock that excludes nothing cannot pass either; it fails at the old seeding line and passes now. --------- Co-authored-by: SanJiu <tlyyxjz@users.noreply.github.com>
1 parent bb9b075 commit e44dd47

2 files changed

Lines changed: 412 additions & 18 deletions

File tree

‎src/powercontext/server/cursor_secret.py‎

Lines changed: 142 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,14 @@
1616

1717
from __future__ import annotations
1818

19+
import contextlib
1920
import os
2021
import secrets
22+
import time
23+
from collections.abc import Callable, Iterator
24+
from contextlib import contextmanager
2125
from pathlib import Path
26+
from typing import cast
2227

2328
from sqlalchemy.engine import make_url
2429

@@ -27,6 +32,8 @@
2732
from powercontext.paths import powercontext_data_dir
2833

2934
_CURSOR_SECRET_BYTES = 32
35+
_KEY_CREATE_ATTEMPTS = 3
36+
_KEY_LOCK_TIMEOUT_SECONDS = 10.0
3037

3138

3239
def resolve_cursor_secret(database: DatabaseConfig, configured_secret: str | None, /) -> bytes | None:
@@ -46,29 +53,147 @@ def resolve_cursor_secret(database: DatabaseConfig, configured_secret: str | Non
4653

4754
def _load_or_create(path: Path) -> bytes:
4855
path.parent.mkdir(parents=True, exist_ok=True)
49-
try:
50-
return _read(path)
51-
except FileNotFoundError:
52-
pass
56+
# Deciding that a key is debris and publishing its replacement is one
57+
# critical section. Without it, a starter that read a missing key can wake
58+
# up after another starter published a complete one, and delete it.
59+
with _creation_lock(path):
60+
return _create_or_reuse_locked(path)
61+
62+
63+
def _create_or_reuse_locked(path: Path) -> bytes:
64+
for _ in range(_KEY_CREATE_ATTEMPTS):
65+
secret = _read(path)
66+
if secret is not None:
67+
return secret
68+
69+
# Only inside the lock can a key of the wrong length be assumed to come
70+
# from a creation that stopped before the writer returned it: every
71+
# writer holds this lock, so no other starter is midway through writing.
72+
# Outside the lock, a short file may belong to a starter that is about
73+
# to publish the key it will sign cursors with.
74+
with contextlib.suppress(OSError):
75+
path.unlink(missing_ok=True)
76+
77+
generated = secrets.token_bytes(_CURSOR_SECRET_BYTES)
78+
try:
79+
# Windows text descriptors translate LF bytes even when using os.write.
80+
flags = os.O_WRONLY | os.O_CREAT | os.O_EXCL | getattr(os, "O_BINARY", 0)
81+
descriptor = os.open(path, flags, 0o600)
82+
except FileExistsError:
83+
# Another starter won the race; use the key it publishes.
84+
continue
85+
completed = False
86+
try:
87+
_write_key(descriptor, generated, path)
88+
os.fsync(descriptor)
89+
completed = True
90+
finally:
91+
os.close(descriptor)
92+
if not completed:
93+
# Do not leave a half-written key behind: the next start would
94+
# have to discard it before it could create a usable one.
95+
with contextlib.suppress(OSError):
96+
path.unlink(missing_ok=True)
97+
return generated
98+
99+
raise OSError(f"failed to create cursor signing key: {path}") # noqa: TRY003
53100

54-
generated = secrets.token_bytes(_CURSOR_SECRET_BYTES)
101+
102+
@contextmanager
103+
def _creation_lock(path: Path) -> Iterator[None]:
104+
"""Hold an exclusive cross-process lock for one cursor key location."""
105+
106+
# Keep the lock inode after release: removing it lets a waiter on the old
107+
# inode and a newcomer on the replacement enter at the same time.
108+
lock_path = path.with_name(f"{path.name}.lock")
109+
if os.name == "nt":
110+
with _windows_creation_lock(lock_path):
111+
yield
112+
return
113+
114+
import fcntl
115+
116+
descriptor = os.open(lock_path, os.O_CREAT | os.O_RDWR | getattr(os, "O_NOFOLLOW", 0), 0o600)
55117
try:
56-
# Windows text descriptors translate LF bytes even when using os.write.
57-
flags = os.O_WRONLY | os.O_CREAT | os.O_EXCL | getattr(os, "O_BINARY", 0)
58-
descriptor = os.open(path, flags, 0o600)
59-
except FileExistsError:
60-
return _read(path)
118+
deadline = time.monotonic() + _KEY_LOCK_TIMEOUT_SECONDS
119+
while True:
120+
try:
121+
fcntl.flock(descriptor, fcntl.LOCK_EX | fcntl.LOCK_NB)
122+
break
123+
except BlockingIOError:
124+
if time.monotonic() >= deadline:
125+
raise OSError( # noqa: TRY003
126+
f"timed out waiting for the cursor signing key lock: {path}"
127+
) from None
128+
time.sleep(0.05)
129+
yield
130+
finally:
131+
with contextlib.suppress(OSError):
132+
fcntl.flock(descriptor, fcntl.LOCK_UN)
133+
os.close(descriptor)
134+
135+
136+
@contextmanager
137+
def _windows_creation_lock(lock_path: Path) -> Iterator[None]:
138+
import msvcrt
139+
140+
# These Windows-only members are missing from the stdlib type stubs.
141+
msvcrt_members = vars(msvcrt)
142+
locking = cast(Callable[[int, int, int], None], msvcrt_members["locking"])
143+
lock_nonblocking = cast(int, msvcrt_members["LK_NBLCK"])
144+
lock_unlock = cast(int, msvcrt_members["LK_UNLCK"])
145+
descriptor = os.open(lock_path, os.O_CREAT | os.O_RDWR, 0o600)
61146
try:
62-
written = os.write(descriptor, generated)
63-
if written != len(generated):
64-
raise OSError(f"failed to write complete cursor signing key: {path}") # noqa: TRY003
147+
# Take the lock without seeding the file first. `_locking` accepts a range
148+
# past the end of the file, and writing a marker before the lock is held is
149+
# itself a race: a starter that read the size before another starter locked
150+
# byte 0 would write into that byte, Windows rejects the write with errno 13,
151+
# and the error escapes the retry loop below, so the second starter never
152+
# starts even though the persisted key is valid.
153+
deadline = time.monotonic() + _KEY_LOCK_TIMEOUT_SECONDS
154+
while True:
155+
try:
156+
os.lseek(descriptor, 0, os.SEEK_SET)
157+
locking(descriptor, lock_nonblocking, 1)
158+
break
159+
except OSError:
160+
if time.monotonic() >= deadline:
161+
raise OSError( # noqa: TRY003
162+
f"timed out waiting for the cursor signing key lock: {lock_path}"
163+
) from None
164+
time.sleep(0.05)
165+
yield
65166
finally:
167+
with contextlib.suppress(OSError):
168+
os.lseek(descriptor, 0, os.SEEK_SET)
169+
locking(descriptor, lock_unlock, 1)
66170
os.close(descriptor)
67-
return generated
68171

69172

70-
def _read(path: Path) -> bytes:
71-
secret = path.read_bytes()
173+
def _write_key(descriptor: int, secret: bytes, path: Path) -> None:
174+
"""Write every byte: os.write may accept fewer bytes than requested."""
175+
176+
remaining = memoryview(secret)
177+
while remaining:
178+
written = os.write(descriptor, remaining)
179+
if written <= 0:
180+
raise OSError(f"failed to write complete cursor signing key: {path}") # noqa: TRY003
181+
remaining = remaining[written:]
182+
183+
184+
def _read(path: Path) -> bytes | None:
185+
"""Return the stored key, or None when there is no complete key on disk.
186+
187+
"No complete key" covers both a missing file and a file of the wrong
188+
length. Treating the second as debris is only safe inside `_creation_lock`:
189+
a short file may also be a key that another starter is still writing and
190+
about to sign cursors with.
191+
"""
192+
193+
try:
194+
secret = path.read_bytes()
195+
except FileNotFoundError:
196+
return None
72197
if len(secret) != _CURSOR_SECRET_BYTES:
73-
raise ValueError(f"cursor signing key must contain {_CURSOR_SECRET_BYTES} bytes: {path}") # noqa: TRY003
198+
return None
74199
return secret

0 commit comments

Comments
 (0)