Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion mempalace/palace.py
Original file line number Diff line number Diff line change
Expand Up @@ -1165,8 +1165,14 @@ def mine_palace_lock(palace_path: str):
) from exc
# Record our own identity for any later contender's diagnostic message.
_write_lock_holder(lf)
_mark_held(palace_key)
# Mark the hold from inside the try so it always pairs with
# _mark_released. If it sat before the try, an async exception
# (a SIGINT/KeyboardInterrupt) landing in the gap would orphan
# palace_key in the holder set while the outer finally frees the
# flock, so the in-memory hold would outlive the OS lock and a later
# re-entrant acquire would pass through and write without the flock.
try:
_mark_held(palace_key)
yield
finally:
_mark_released(palace_key)
Expand Down
45 changes: 45 additions & 0 deletions tests/test_palace_locks.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import pytest

import mempalace.palace as palace_mod
from mempalace.palace import (
_write_lock_holder,
MineAlreadyRunning,
Expand Down Expand Up @@ -394,3 +395,47 @@ def test_mine_global_lock_is_alias_for_back_compat(tmp_path, monkeypatch):
assert mine_global_lock is mine_palace_lock
with mine_global_lock(str(tmp_path / "palace")):
pass # the alias accepts the same palace_path argument


def test_holder_set_not_orphaned_by_interrupt_after_mark_held(tmp_path, monkeypatch):
"""An async interrupt right after the hold is recorded must not leave the
palace key stranded in the process-wide holder set.

``_mark_held`` sits inside the ``try`` whose ``finally`` runs
``_mark_released``, so the two are paired on every exit. If ``_mark_held``
ran before the ``try``, a ``KeyboardInterrupt``/signal landing in the gap
would strand the key: the outer ``finally`` still frees the flock, so
``_held_by_this_process`` would report a hold the OS lock no longer backs,
and the next re-entrant acquire in this process would pass through and
write without the flock (two concurrent writers into one palace).
"""
monkeypatch.setenv("HOME", str(tmp_path))
palace = str(tmp_path / "palace")

before = set(palace_mod._palace_lock_keys)

# Model the interrupt: run the real _mark_held (records the hold), then
# raise, as a signal arriving at that instant would.
real_mark_held = palace_mod._mark_held

def _mark_then_interrupt(lock_key):
real_mark_held(lock_key)
raise KeyboardInterrupt

palace_mod._mark_held = _mark_then_interrupt
try:
with pytest.raises(KeyboardInterrupt):
with mine_palace_lock(palace):
pass
finally:
# Restore by hand (not monkeypatch.setattr, which unpatches only at
# teardown) so the reuse check below calls the real _mark_held.
palace_mod._mark_held = real_mark_held

assert set(palace_mod._palace_lock_keys) == before, (
"palace key was stranded in the holder set after an interrupt: "
"the in-memory hold outlived the flock"
)
# The flock was freed and no stale hold remains, so the lock is reusable.
with mine_palace_lock(palace):
pass
Loading