Concurrent ModelManager mutations can split lock generations and desynchronize fixed-size cache bookkeeping #2819

Search before asking

I searched the open and closed issues, pull requests, and current upstream
branches for ModelManager, WithFixedSizeCache, per-model lock generations,
concurrent add/remove, alias handling, duplicate LRU entries, and removal
rollback.

I found the historical locking work in #1512, but no issue or pull request that
preserves one per-model lock generation across all existing callers and keeps
the fixed-size cache bookkeeping consistent across the mutation failures below.

Bug

On current main at
aa9306bd8e3c674c3c0a324635307b7272eb54ef, process-local mutations for one
resolved model identifier are not governed by one stable lock generation.

The base-manager race is:

1. remove(X) acquires per-model lock L1.
2. add_model(X) obtains a reference to L1 and waits.
3. remove(X) deletes the model and removes _models_state_locks[X] while it
   still holds L1.
4. A new add_model(X) sees no lock-registry entry, creates L2, and starts
   initialization.
5. remove(X) releases L1.
6. The original waiter acquires L1 and starts a second initialization.

Two constructors for the same resolved identifier can therefore run
concurrently.

The surrounding decorators expose related mutation-lifecycle inconsistencies:

• when remove(X) already owns the per-model lock, a later warm
  ModelManagerDecorator.add_model(X) can bypass that established mutation
  order through an unlocked existence check and return before removal completes
• WithFixedSizeCache.remove() removes the LRU entry before the underlying
  clear_cache() succeeds; if removal fails, the identifier remains registered
  in the manager but disappears from the queue
• eviction has the same rollback problem when clear_cache() raises
• concurrent cold adds for one resolved identifier can append duplicate LRU
  entries
• the outer fixed-size cache resolves model_id_alias, while the generic
  decorator can still return based on the raw model ID, queuing an alias that
  was never loaded

The standard WithFixedSizeCache(ModelManager(...)) composition reaches these
paths.

Expected behavior

For one resolved model identifier, process-local load and removal mutations
should share one serialized lifecycle:

• a lock generation remains registered from caller reservation through holder,
  waiter, timeout, and cleanup completion
• a new generation is created only after all callers associated with the
  previous generation have finished
• decorators preserve the mutation ordering of the wrapped ModelManager
• after a completed or failed add_model() or remove() operation, the LRU
  queue contains at most one entry for the resolved identifier and agrees with
  whether that identifier remains registered in the underlying manager
• the lifecycle-entry guard is not held while waiting on a per-model lock,
  loading a model, or clearing model resources

Minimal reproducible examples

The schedules use threading.Event; no timing sleeps are required.

Decorator bypasses an established remove/add order

remove_thread.start()
assert removal_started.wait(timeout=1)

add_thread.start()

# On current main the decorator returns while remove still owns L1.
assert add_returned.wait(timeout=1)
assert not removal_finished.is_set()

allow_remove.set()
remove_thread.join(timeout=1)
add_thread.join(timeout=1)

assert X in manager

The assertion fails on main: add_model() returned before the already
in-progress removal, and the identifier is absent after both operations.
The wrapped base manager would instead wait for L1, recheck after removal,
and load X.

Fixed-size cache rollback

manager = WithFixedSizeCache(
    ModelManager(registry, models={X: ModelWhoseClearCacheRaises()}),
    max_size=1,
)

with pytest.raises(RuntimeError):
    manager.remove(X)

assert X in manager
assert list(manager._key_queue) == [X]

The final assertion fails on main: the underlying manager still registers
X, but the LRU entry was removed before clear_cache() failed.

The lock-generation regression additionally pauses remove(X) after deleting
the lock-registry entry but before releasing L1, starts both an existing
waiter and a newcomer, and observes two simultaneously active constructors.

Environment

• Inference: main@aa9306bd8e3c674c3c0a324635307b7272eb54ef
• Python: 3.12.13
• OS: macOS / Apple Silicon
• Hardware requirements: CPU only; no model weights or GPU involved

Additional scope notes

• Demonstrated impact: two live lock generations, loss of same-key mutual
  exclusion, concurrent constructors, decorator behavior inconsistent with the
  wrapped manager's mutation ordering, duplicate queue entries, and
  LRU/manager-registration disagreement
• Plausible downstream impact: duplicate download or deserialization, temporary
  RAM or VRAM spikes, GPU OOM, extra disk/network work, and last-write-wins
  model replacement; no production incident or GPU OOM was reproduced
• The intended guarantee is process-local to one ModelManager instance
• Active inference-use leases, eviction protection for models already in use,
  strict global max_size enforcement across concurrent different-key loads,
  fairness/starvation, and cross-process coordination are outside this report

Are you willing to submit a PR?

Yes. I have a local implementation with deterministic parent/candidate
coverage for lock generations, decorator mutation ordering, aliases, queue
deduplication, removal and eviction rollback, multiple waiters, timeout
cleanup, and different-identifier progress.