An interrupted `SmartLock.acquire()` leaves a ghost waiter and poisons the lock

[pomponchik]

When a thread waits for a busy SmartLock or SmartRLock, it is added to the internal queue and wait-for graph before the blocking wait starts.

If that wait is interrupted by KeyboardInterrupt, SystemExit, or another exception, acquire() exits without removing the waiter. The lock may later treat that thread as its owner even though the acquisition failed.

This can permanently block later callers and retain an unbounded number of waiter records, graph edges, blocked threads, and thread stacks.

Severity

• Correctness and availability: High
• Memory and resource retention: Medium
• Confidence: High

Both SmartLock and SmartRLock are affected because the problem is in AbstractSmartLock.acquire().

Expected behavior

If a contended acquire() raises before ownership is transferred:

• the interrupted thread must not become the owner;
• its waiter state and graph edge must be cleaned safely;
• later waiters must not remain blocked behind it;
• the lock must remain reusable after the real owner releases it.

The caller should not need to call release() after a failed acquire().

Actual behavior

The interrupted thread remains in:

• deque;
• local_locks;
• recursion_depths;
• temporarily, the wait-for graph.

After the real owner releases the lock, the interrupted thread becomes a ghost owner. Its private hand-off lock remains locked forever, so later waiters can block permanently.

Root cause

The waiter is fully registered in locklib/locks/smart_lock/abstract.py:54-60:

previous_element = self.deque[0]
self.graph.add_link(thread_id, previous_element)
self.deque.appendleft(thread_id)
self.local_locks[thread_id] = Lock()
self.local_locks[thread_id].acquire()
self.recursion_depths[thread_id] = 1
previous_element_lock = self.local_locks[previous_element]

The actual blocking wait happens later, on line 63:

previous_element_lock.acquire()

There is no cancellation or rollback if this call raises.

Failure sequence

Assume thread A owns the lock and thread B is waiting:

deque = [B, A]
graph = {B: {A}}

Thread B is blocked on A's private hand-off lock. Its own private lock is already created and locked.

If B is interrupted, B.acquire() raises, but its internal state remains unchanged.

When A later releases the lock, the state becomes:

deque = [B]
local_locks = {B: locked_gate_B}
recursion_depths = {B: 1}
graph = {}

The implementation now considers B the owner, although B.acquire() failed.

If thread C calls acquire(), it is queued behind B and waits on gate_B. No thread will release that gate, so C can remain blocked forever.

The same problem affects with lock:. If __enter__() raises, Python does not call __exit__(), so cleanup must be performed by the lock implementation.

Impact

One interrupted acquisition leaves a small amount of stale per-lock state. The larger problem appears when the application continues using the poisoned lock.

Each later waiter can add:

• another queue entry;
• another private threading.Lock;
• another recursion-depth entry;
• another graph edge;
• a blocked thread and its native stack;
• objects referenced by that thread's Python frames.

The result is both a permanent lock failure and potentially unbounded resource retention.

This is not the usual error of acquiring a lock without releasing it: the affected acquire() never succeeded.

Regression test

The following deterministic test reproduces the failed-wait path without depending on operating-system signal behavior.

Suggested location:

tests/units/locks/smart_lock/test_interrupted_acquire.py

from threading import Event, Thread
from typing import Any, List, Type

import pytest

from locklib.locks.smart_lock.abstract import AbstractSmartLock
from locklib.locks.smart_lock.graph import LocksGraph


class InjectedAcquireInterruption(BaseException):
    pass


class InterruptingGate:
    def __init__(self, wrapped_gate: Any) -> None:
        self.wrapped_gate = wrapped_gate

    def acquire(self) -> None:
        raise InjectedAcquireInterruption

    def release(self) -> None:
        self.wrapped_gate.release()


@pytest.mark.timeout(5)
def test_interrupted_waiter_is_removed_and_lock_remains_reusable(
    smartlock_class: Type[AbstractSmartLock],
) -> None:
    graph = LocksGraph()
    lock = smartlock_class(local_graph=graph)

    owner_is_ready = Event()
    owner_may_release = Event()
    owner_errors: List[BaseException] = []

    def owner() -> None:
        try:
            lock.acquire()
            owner_is_ready.set()
            owner_may_release.wait()
            lock.release()
        except BaseException as error:  # noqa: BLE001
            owner_errors.append(error)

    owner_thread = Thread(target=owner, daemon=True)
    owner_thread.start()
    assert owner_is_ready.wait(1)

    # Replace the owner's hand-off lock with a wrapper that simulates an
    # interrupted blocking acquire while preserving the owner's release.
    with lock.lock, graph.lock:
        owner_thread_id = lock.deque[-1]
        owner_gate = lock.local_locks[owner_thread_id]
        lock.local_locks[owner_thread_id] = InterruptingGate(
            owner_gate,
        )  # type: ignore[assignment]

    try:
        with pytest.raises(InjectedAcquireInterruption):
            lock.acquire()
    finally:
        owner_may_release.set()
        owner_thread.join(1)

    assert not owner_thread.is_alive()
    assert not owner_errors

    # These assertions fail on the current implementation because the
    # interrupted waiter remains as a ghost owner.
    with lock.lock, graph.lock:
        assert not lock.deque
        assert not lock.local_locks
        assert not lock.recursion_depths
        assert graph.links == {}

    # The same instance must remain usable after cleanup.
    lock.acquire()
    lock.release()

    with lock.lock, graph.lock:
        assert not lock.deque
        assert not lock.local_locks
        assert not lock.recursion_depths
        assert graph.links == {}

The existing smartlock_class fixture runs this test for both SmartLock and SmartRLock.

Test behavior

Before the fix:

• the test fails for both lock classes;
• deque, local_locks, and recursion_depths contain the ghost waiter.

After the fix:

• cancelled waiter state is cleaned safely;
• the graph is empty;
• the lock can be acquired and released again;
• the test passes for both lock classes.

A separate POSIX integration test using SIGALRM may be added to verify a real interruption of threading.Lock.acquire(), but the deterministic test should remain the primary cross-platform regression test.

Acceptance criteria

• An exception escaping from a contended acquire() never makes that thread an owner.
• Cancelled waiter state is removed without breaking waiters queued behind it.
• The wait-for graph does not retain a stale edge.
• The real owner can still release normally.
• Later callers do not hang behind a ghost owner.
• The lock remains reusable.
• Regression coverage includes both SmartLock and SmartRLock.
• The full test suite continues to pass with 100% line and branch coverage.

Implementation note

A simple except BaseException followed by deleting the interrupted waiter may not be sufficient. Another thread may already be queued behind it and waiting on its private hand-off lock.

The fix needs cancellation-aware ownership transfer so that queue state, graph state, and private hand-off locks remain consistent even when an exception interrupts the wait.

Environment

• locklib: 0.0.25
• Python: 3.11.9
• Operating system: macOS 15.3.1 (build 24D70)