Skip to content

fix(metrics): render_prometheus deadlock if self.fallback_rate called inside threading.Lock #254

Closed

Description

@piyushmakhija28

Summary

ProductMetrics.render_prometheus() acquires self._lock and snapshots values inline rather than calling self.fallback_rate (the property). This is an undocumented defensive pattern: self._lock is a non-reentrant threading.Lock, so calling self.fallback_rate inside the lock would deadlock. Any future refactor that replaces the inline computation with self.fallback_rate will ship a production deadlock under concurrent Prometheus scraping.

Root Cause

backend/app/productization/metrics.py around line 134:

with self._lock:
    # snapshot values manually — cannot call self.fallback_rate here
    # because fallback_rate property also acquires self._lock → deadlock
    routing_count = self.routing_count
    fallback_count = self.fallback_count
    fallback_rate = fallback_count / routing_count if routing_count else 0.0  # inline

The fallback_rate property acquires self._lock. render_prometheus also holds self._lock. Non-reentrant lock → deadlock. The current code avoids this by duplicating the computation inline, but there is no comment explaining why, making the constraint invisible to future contributors.

Failure Scenario

  1. Developer adds: fallback_rate = self.fallback_rate inside render_prometheus()'s with self._lock: block.
  2. Prometheus scraper calls /metrics concurrently with a routing request calling record_routing().
  3. render_prometheus acquires self._lock, then attempts self.fallback_rate which tries to acquire self._lock again.
  4. Deadlock. The /metrics endpoint hangs forever. The routing request is also blocked on self._lock.
  5. Worker appears healthy (no crash) but all requests queue behind the deadlocked lock.

Proposed Fix

Switch to threading.RLock (reentrant lock) to make the property callable from within the lock:

_lock: threading.RLock = field(default_factory=threading.RLock)

And add a comment explaining the constraint:

# NOTE: _lock is RLock so properties like fallback_rate can be called
# safely from within render_prometheus() without deadlocking.

Or replace the property pattern with a private method that does NOT acquire the lock, callable only from already-locked contexts.

Activity

  1. added
    bugSomething isn't working
    priority: highData loss, security, or major correctness bug
    code-reviewFound via automated code review
    on Jun 21, 2026
  2. piyushmakhija28 commented on Jun 21, 2026

    @piyushmakhija28
    ContributorAuthor

    Fixed in PR #280 (Sprint R3/R4: fix(sprint-r3): root-cause fixes for all 39 bugs from code review round 3 + Sprint R4 round 4)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingcode-reviewFound via automated code reviewpriority: highData loss, security, or major correctness bug

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions