| 文件 | 最后提交记录 | 最后更新时间 |
|---|---|---|
fix(model cache): evict records at shutdown() instead of only releasing shared weights (#9494) * fix(model cache): release shared weights when a cache goes away Nothing released a cache's SharedCpuWeightsStore references except _delete_cache_entry(): shutdown() left every resident record's refcount held, and a cache dropped without shutdown() (test teardown; any future wiring that rebuilds caches at runtime) stranded the canonical tensors and their accounting forever. Today's production wiring tears the store down together with its caches, so the live exposure is cross-test pollution of the process-global store and RAM pinned past ModelManagerService.stop() — but the refcount invariant ('every acquire is paired with exactly one release') was simply not upheld, and this makes it self-healing before any wiring change turns it into a real peer-accounting bug. Two mechanisms, for the two ways a cache goes away: - shutdown() now releases its resident records' shared references synchronously — it runs in a normal thread context, so the direct (locking) release is safe there, and teardown does not depend on a later store operation happening. - Each wrapper registers a weakref.finalize fallback for the dropped-without-shutdown case. The finalizer runs in GC context, where taking the store's non-reentrant lock could self-deadlock (a collection can fire inside acquire()'s critical section on the same thread — the rule ModelCache.release_first_use_grace documents), so it only ENQUEUES into a SimpleQueue; every public store method drains the queue under the lock. The finalizer is registered inside the acquire's try (a registration failure must release too), its args carry the key and canonical dict rather than the wrapper (finalize holds args strongly — referencing self would make the wrapper immortal), and release_shared_weights() detaches it before releasing synchronously so eviction-then-collection releases exactly once. The state-dict identity keeps releases correct across invalidate()'s retired entries. RamBudget.total_in_use() now documents why its store read must stay outside the budget lock: the drain allocates under the store lock, so GC can run _on_cache_collected (store→budget) there, and a budget→store order anywhere would complete the deadlock cycle. Six regression tests, verified to fail before the fix, covering: shutdown releases synchronously with an empty queue; collection returns refcount/bytes/budget to zero; the collection-time release is enqueue-only (never applied inline by GC); eviction + collection release exactly once across two caches; a retired (invalidated) entry is freed by a collected holder; and the partial-load wrapper behaves like the full-load one. One existing test relied on an abandoned wrapper leaking its reference and now binds it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(model cache): evict records at shutdown() instead of only releasing shared weights shutdown() released the resident records' shared-store references while retaining the records themselves, so the accounting stopped describing reality: - The store (and RamBudget) reported zero for bytes whose tensors the retained wrappers still held. - A post-shutdown load of the same key on a peer cache registered a duplicate canonical alongside the still-resident released copy. - A post-shutdown eviction of a released record (put() after shutdown() is reachable: Invoker.stop() stops the model manager before the session processor) read uses_shared_weights as already-False and debited the non-shared budget for bytes that were admitted as shared. shutdown() now routes idle records through _delete_cache_entry(), which releases shared ownership and budget accounting together, exactly once. Records still in use — locked by an in-flight generation or inside the put()->lock() admission window — keep their references and are marked stale; unlock() evicts them through the existing stale path when the generation lets go, so the accounting stays truthful at every point. All five regression tests verified to fail against the previous shutdown() behavior. Follow-on to #9403, addressing JPPhoto's review comment there. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(model cache): match records by identity in stale eviction and _delete_cache_entry Surfaced by adversarial review of the shutdown() change: a stale-marked record can be detached while still locked (the VRAM-move error paths call _delete_cache_entry on a locked record) and its key re-admitted before the record's last unlock(). The stale-eviction path matched by key only, so it popped the NEW record — detaching it from the cache and all accounting — and, the old record's shared release having already happened, read uses_shared_weights as False and debited the non-shared budget for bytes that were admitted as shared. The hazard predates the shutdown() change (drop_model() sets the same flag), but shutdown() now arms stale marks at every server stop that overlaps in-flight work, so close it here: _delete_cache_entry() and unlock()'s stale eviction act only when the record passed in IS the record currently held under its key; a delete of a detached record is a full no-op. Regression test verified to fail against the key-only matching. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(model cache): track get()->lock() holders through shutdown and abandonment Two defects found in review of the shutdown eviction change (JPPhoto, 2026-08-13): 1. shutdown() racing the gap between get() and the LoadedModel's first lock evicted the warm record out from under its holder: the holder locked a detached record whose shared-store ownership had just been released, so a peer's reload of the same key minted a duplicate canonical copy while the budget counted one. 2. A record retained by the shutdown sweep for a never-locked holder could never be evicted if that holder was simply dropped: the abandonment finalizer's deferred work was discarded post-shutdown (and the worker was stopped), pinning the record, its shared-store refcount and its budget bytes for the life of the process. The fix tracks every wrapper's get()->lock() window with a per-record hold count (CacheRecord.first_use_holds), armed in LoadedModelWithoutConfig's constructor and released exactly once per wrapper — on its first lock, or by its weakref finalizer if it is dropped un-entered. Held records are treated like locked ones by every eviction path (shutdown, budget reconcile, peer-requested eviction, make_room, drop_model, unlock's stale eviction); stale-marked records whose last holder is abandoned are evicted by the deferred worker, which now outlives shutdown() for exactly that purpose (it already exits via the cache-collection finalizer). Holds are only granted while a worker is alive to carry the finalizer's release, and a worker death zeroes surviving holds at the next start so no record can stay shielded with nothing left to unshield it. Admissions landing after shutdown() are marked stale at birth so their final release evicts them too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(model cache): epoch-guard hold releases and recover stranded holds at shutdown Hardening from adversarial review of the first-use-hold mechanism: - Hold releases (the wrapper's first-lock release and the abandonment finalizer's deferred release) now quote the epoch the hold was armed under, and dead-worker recovery bumps the record's epoch when it zeroes stranded holds. Without this, a surviving wrapper's late release — or a release enqueued before the worker died and drained after the restart — would decrement a fresh hold armed by a different wrapper under the healthy replacement worker, silently unshielding that wrapper's window. - shutdown() now runs the dead-worker hold recovery itself (and clears the put()-grace flags in the same situation): a hold whose abandonment release was dropped by the dead-thread dispatch check has no other releaser, and after shutdown no put() is guaranteed to run the usual next-start recovery — the sweep would stale-retain the record, its shared-store refcount and its budget bytes for the life of the process. - register_first_use_hold() declines to arm on a record that is no longer the occupant under its key: an eviction already won the race against the wrapper's construction and a hold on a detached record shields nothing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(model cache): withhold the post-shutdown grace and recover from the worker's own death Two follow-ups from review. put() after shutdown() no longer arms the post-admission grace. That flag's backstop releaser is the sweep at the top of the next put(), and after shutdown no further put() is guaranteed: a load cancelled between put() and the LoadedModel's construction leaves no wrapper (hence no finalizer either), so an armed flag would stand for the life of the process, hiding the record from every asynchronous eviction path while its bytes stayed charged to the shared budget. Withholding it costs only the shield -- the record stays stale at birth, so its eventual release still evicts it, and a loader that does come back gets the ordinary first_use_holds shield. The deferred worker now runs stranded-shield recovery from inside its own dying frame. Previously recovery depended on something else happening first -- the next admission, or shutdown() -- and neither is guaranteed when the worker dies *after* shutdown()'s liveness check: the records the shutdown sweep retained for a live holder were left shielded by holds nothing could release. The recovery is scoped by thread identity (a replacement worker's shields are its own) and retires the worker slot before sweeping, so a concurrent admission cannot arm a shield the recovery is about to zero. It also drains the queue the dead worker left behind, whose _AbandonedHolderRelease items pin their models' CPU weights. _ensure_deferred_worker and shutdown() now share the same recovery, which also lifts orphaned admission graces and evicts whatever that leaves unshielded. Three tests, each reverted-and-confirmed-failing against the code it guards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GjAw9hpJe8d1GyvpdKJFGw * fix(model cache): narrow the dead-worker recovery and stop it pinning records Adversarial review of the previous commit found three problems with the shared recovery it introduced. The recovery cleared the put()-set admission grace unconditionally. On a live cache that is a new failure mode, not a fix: the dying worker is is_alive() for as long as it unwinds, so a cold load landing in that window starts no replacement worker and is admitted with the ordinary grace, which the recovery then zeroed while the loader was still between put() and get() -- a reconcile could evict the record and the loader's get() would raise IndexError. The grace only actually loses a releaser once the cache is shut down (its backstop is the next put()'s sweep, not the worker), so it is now lifted only then. The recovery also evicted stale-unshielded records from _ensure_deferred_worker, which register_first_use_hold calls before arming -- so a second wrapper's construction could detach the very record it was about to shield, releasing shared-store ownership while live wrappers still held the tensors. That is the accounting lie shutdown() itself refuses to make. The eviction moved to _evict_stale_unshielded_entries, called only from the dying worker and only on a shut-down cache, where nothing else can ever run it; it now also collects and empties the device cache the way the other abandonment path does. Keeping shutdown()'s call to pure field assignments restores its old property that the branch cannot raise before the resident-record sweep. The queue drain the previous commit added did not close the pin it targeted: _dispatch_deferred's liveness gate is unsynchronized, so a finalizer that read the worker slot just before it was retired still enqueues after the drain. The drain is gone; _AbandonedHolderRelease now holds its record weakly instead, so a stranded item pins nothing, and the worker clears the strong reference it resolves before parking on the next get(). Also moves _reconcile_budget_if_pending's lock acquisition adjacent to its try: a BaseException between the two leaked the cache RLock to an unwinding thread, blocking every other thread for the life of the process. Five tests, each reverted-and-confirmed-failing against the code it guards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GjAw9hpJe8d1GyvpdKJFGw * fix(model cache): key the dead-worker backstops on worker liveness, not the slot A second adversarial pass found that retiring the worker slot from inside the dying worker silently disabled both remaining recovery sites, which gated on "a dead thread still occupying the slot". The dying recovery deliberately leaves a live cache's admission grace standing -- the next put()'s sweep is still its backstop -- and hands the lift to shutdown(); with the slot already empty, shutdown() skipped it and stale-retained the record, its shared-store reference and its budget bytes for the life of the process. Both gates now key on "no live worker": shutdown() lifts when the slot is empty or dead, and the worker start recovers unconditionally (it has already returned if a worker is alive). That also makes a failed recovery retryable, which matters because the recovery was not exception-safe and had already retired the slot by the time it could raise. _clear_stranded_first_use_holds now unshields every record before reporting any of them -- a logging handler that raises is one of the ways the worker dies in the first place, and logging inline let that same handler abort the sweep partway -- and the post-eviction gc/empty_cache housekeeping, which the codebase already documents can raise from a sick CUDA context, no longer takes the eviction down with it. Also corrects two overstated claims in the weakref rationale: a stranded queue item can be drained later by a replacement worker (the queue is per-cache, not per-worker), and the hold decrement in _release_abandoned_holder runs before the identity check -- it is inert on a detached record for a different reason, which the docstring now gives. Moving _reconcile_budget_if_pending's acquire adjacent to its try narrows the RLock-leak window rather than closing it; said so. Three tests, each reverted-and-confirmed-failing against the code it guards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GjAw9hpJe8d1GyvpdKJFGw * fix(model cache): claim the first-use window at the lookup, and let a cancelled admission release itself Two findings from review. The first-use shield was armed by LoadedModel's constructor, leaving the whole stretch between the cache lookup and that constructor unshielded -- and that stretch is not a couple of instructions: the configured loader retrieves its record inside _load_and_cache and then does the shared-store shell registration and two returns before load_model wraps it. A shutdown sweep or a peer's reconcile landing there detached the record its holder was about to lock, releasing shared-store ownership while the tensors lived on, so a peer's reload minted a duplicate canonical the budget counted once. ModelCache.get_with_first_use_claim() now arms the hold in the same lock acquisition as the lookup and hands back a FirstUseClaim that owns it: the wrapper adopts the claim and releases it at its first lock, and a claim dropped without ever being adopted -- the load raised before a wrapper existed -- releases the hold by dying. shutdown() stale-retained a record carrying only the put()-set admission grace. That grace's three releasers are the loader's own get()->lock(), the abandonment finalizer of a wrapper built from the record, and the sweep at the top of the next put(); a load cancelled between its put() and its retrieval has neither of the first two, and after shutdown no further put() is guaranteed to run the third, so the record, its shared-store reference and its budget charge stood until the cache object was collected. put(claim_admission=True) now hands the loader a claim over that window too, so such a load releases its admission by dying and the shutdown sweep finds an ordinary idle record. Retiring the grace at shutdown instead -- the obvious shortcut, and what the first two drafts of this commit did -- is not safe. The flag is unowned, so a standing grace does not mean nobody is working on the record: it is equally the state of a load still between its put() and its retrieval, and of a live un-entered wrapper whose hold a worker death zeroed. Both were evicted out from under their holder, with the duplicate-canonical accounting lie and an IndexError from a retrieval that no longer found its own model. For the same reason the admission window is not shielded by either flag but by a weak reference to the claim (CacheRecord.admission_claim_ref): nothing has to release it, so neither a worker death (which zeroes holds) nor another holder's abandonment (which clears the grace) can make a running load look finished. _recover_stranded_shields retires it once the cache is shut down, where the eviction its expiry should trigger would otherwise travel through a dead worker -- the same trade that method already makes for holds. The claim is armed only after put() has committed its accounting, so a failure to allocate it cannot leave a resident, store-owning record the budget never counted, and both hand-back guards release the hold when the object that was to carry its release cannot be built. Nine tests, each reverted-and-confirmed-failing against the code it guards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FCitU6EFcp76AaauNfaWTz * fix(model cache): refuse post-shutdown prefetch admissions, and keep abandonment releases Two records could survive a shut-down cache with nothing left to retire them. A prefetch=True admission after shutdown() was marked stale and inserted anyway. prefetch is the promise that no loader will come back for the record, so there is no get() -> lock() -> unlock() to run the stale eviction, no wrapper whose finalizer could carry an abandonment release, and no claim whose expiry could stand in for either. What is left are the paths that may or may not run -- another admission's make_room, a budget reconcile, a peer's eviction request -- and after shutdown none of them is guaranteed to come. put() now refuses such an admission, the same standard the post-admission grace is already withheld under. It costs only a reload: the sole caller takes the submodel it asked for from the pipeline object, not from the cache. The refusal sits after _ensure_deferred_worker() (a post-shutdown prefetch must still revive the worker that carries the retained records' abandonment releases), after the stale-grace sweep (on a shut-down cache with a LIVE worker that sweep is the only backstop a stale grace has left, since shutdown() runs the recovery only when no worker is alive), and before _make_room_internal (nothing resident should be evicted to house a model this call is about to refuse). _dispatch_deferred dropped every item while no worker was running, which included _AbandonedHolderRelease. Its holder is already gone -- finalizers fire once -- so no lock, no unlock and no second finalizer is coming, and that item is the only thing left that can retire the record. Dropping it stranded a record the shutdown sweep had retained, resident and charged, for the life of the process; no later sweep can repair that, because once dead-worker recovery zeroes the hold, a record whose holder is gone is indistinguishable from one a live wrapper is still holding, where retention is required. Abandonment releases are now kept and drained by whichever worker runs next; reconciles are still dropped, since cached_model_keys() can request one on every call and the next cache operation's release hook re-runs it anyway. Evicting from _ensure_deferred_worker() is deliberately NOT the fix: it runs _recover_stranded_shields() immediately before, so at that instant a record a live wrapper still holds looks unshielded, and evicting it would release shared-store ownership while the tensors live on. Four tests, each checked for sensitivity by reverting the line it guards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015n81CKSi7bUA217L2E9SwN * fix(model cache): bound the abandonment queue, own worker-less admissions, keep live claims through recovery JPPhoto's round-6 findings on the shutdown accounting, all three confirmed: - A grace-only abandonment enqueued one kept item per dropped wrapper while no worker could be started, and nothing short of a lock() or another put() ever cleared the grace that kept them coming, so a warm get/drop loop grew the queue without bound. release_first_use_grace now clears the grace itself, lock-free (the flag is monotonic), queues an eviction only for a stale record and only once per record (CacheRecord.abandonment_release_pending, re-opened by the worker the moment it dequeues the item), and still wakes the worker for a pending budget reconcile the drained item used to run. - A post-shutdown put(claim_admission=True) with no startable worker got no claim, so nothing owned the record once its loader died and no later admission swept it. _claim_first_use now mints a hold-less FirstUseClaim (hold_epoch=None) whenever the record is still the occupant, so the admission stays owned through CacheRecord.admission_claim_ref and its finalizer still queues the eviction a stale record owes; and put() runs _evict_stale_unshielded_entries() when the cache is shut down and no worker is alive after its revival attempt, the same terminal sweep the dying worker runs, placed after the grace sweep and before the prefetch refusal. - _recover_stranded_shields retired a live admission claim on a shut-down cache, so a worker death followed by shutdown() evicted a record whose loader was between put() and get(), and that loader's retrieval raised IndexError. A live claim now survives every recovery; the eviction owed once it dies travels through the kept queue item or the next admission's sweep. Two further defects surfaced by the adversarial passes over the fix: shutdown() now marks a record stale BEFORE consulting its shield, so a hold-less abandonment racing the sweep either sees the mark and queues the eviction or has already cleared its shield when the sweep looks; and the coalescing gate is opened at the dequeue site rather than in the handler, so a raise in the handler, a worker death inside it, or a fork cannot leave it closed with nothing queued behind it. Ten new tests, two rewritten; each production change reverted individually and confirmed to fail only the tests that guard it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011JbML8Y8mHdXH72bcUry9o * fix(model cache): refuse worker-less post-shutdown admissions JPPhoto's round-7 blocker, the residual I disclosed rather than closed last round: a claimed admission into a shut-down cache with no startable worker has no releaser (no worker to drain the eviction its dropped claim's finalizer queues) and no guaranteed future cache operation to stand in for one, so a load that dies before retrieving it pins the record, its shared-store reference and its budget bytes for the life of the cache object. put() now refuses every admission -- claimed, plain, and prefetch -- once the cache is shut down and no worker is alive after its revival attempt, the same standard the post-shutdown prefetch was already refused under, generalized. Nothing that could be stranded is admitted. The refusal is narrowly scoped: a live cache still admits worker-less (a future op cleans up), and a normal shutdown keeps its worker alive so the graceful retain-and-reclaim path is unchanged -- only thread exhaustion reaches the refusal, where a load racing shutdown is failing regardless (its retrieval raises IndexError, as a refused prefetch's does; both loaders already handle a None put()). The terminal sweep (_evict_stale_unshielded_entries) still runs before the refusal returns, so a record already orphaned by a worker death is reclaimed even though this admission -- possibly the last cache operation -- is refused. Three tests whose orphan came from the now-refused admission are removed; their mechanism coverage survives elsewhere. Two added: the refusal with its clean loader-style IndexError, and the terminal-sweep-still-runs case. Both new production lines mutated and confirmed to fail only their guarding tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011JbML8Y8mHdXH72bcUry9o * fix(model cache): clean shutdown-retained records --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Jonathan <34005131+JPPhoto@users.noreply.github.com> Co-authored-by: JPPhoto <jpollack@jpollackphoto.com> | 1 小时前 | |
feat(api): chore: pydantic & fastapi upgrade Upgrade pydantic and fastapi to latest. - pydantic~=2.4.2 - fastapi~=103.2 - fastapi-events~=0.9.1 **Big Changes** There are a number of logic changes needed to support pydantic v2. Most changes are very simple, like using the new methods to serialized and deserialize models, but there are a few more complex changes. **Invocations** The biggest change relates to invocation creation, instantiation and validation. Because pydantic v2 moves all validation logic into the rust pydantic-core, we may no longer directly stick our fingers into the validation pie. Previously, we (ab)used models and fields to allow invocation fields to be optional at instantiation, but required when `invoke()` is called. We directly manipulated the fields and invocation models when calling `invoke()`. With pydantic v2, this is much more involved. Changes to the python wrapper do not propagate down to the rust validation logic - you have to rebuild the model. This causes problem with concurrent access to the invocation classes and is not a free operation. This logic has been totally refactored and we do not need to change the model any more. The details are in `baseinvocation.py`, in the `InputField` function and `BaseInvocation.invoke_internal()` method. In the end, this implementation is cleaner. **Invocation Fields** In pydantic v2, you can no longer directly add or remove fields from a model. Previously, we did this to add the `type` field to invocations. **Invocation Decorators** With pydantic v2, we instead use the imperative `create_model()` API to create a new model with the additional field. This is done in `baseinvocation.py` in the `invocation()` wrapper. A similar technique is used for `invocation_output()`. **Minor Changes** There are a number of minor changes around the pydantic v2 models API. **Protected `model_` Namespace** All models' pydantic-provided methods and attributes are prefixed with `model_` and this is considered a protected namespace. This causes some conflict, because "model" means something to us, and we have a ton of pydantic models with attributes starting with "model_". Forunately, there are no direct conflicts. However, in any pydantic model where we define an attribute or method that starts with "model_", we must tell set the protected namespaces to an empty tuple. ```py class IPAdapterModelField(BaseModel): model_name: str = Field(description="Name of the IP-Adapter model") base_model: BaseModelType = Field(description="Base model") model_config = ConfigDict(protected_namespaces=()) ``` **Model Serialization** Pydantic models no longer have `Model.dict()` or `Model.json()`. Instead, we use `Model.model_dump()` or `Model.model_dump_json()`. **Model Deserialization** Pydantic models no longer have `Model.parse_obj()` or `Model.parse_raw()`, and there are no `parse_raw_as()` or `parse_obj_as()` functions. Instead, you need to create a `TypeAdapter` object to parse python objects or JSON into a model. ```py adapter_graph = TypeAdapter(Graph) deserialized_graph_from_json = adapter_graph.validate_json(graph_json) deserialized_graph_from_dict = adapter_graph.validate_python(graph_dict) ``` **Field Customisation** Pydantic `Field`s no longer accept arbitrary args. Now, you must put all additional arbitrary args in a `json_schema_extra` arg on the field. **Schema Customisation** FastAPI and pydantic schema generation now follows the OpenAPI version 3.1 spec. This necessitates two changes: - Our schema customization logic has been revised - Schema parsing to build node templates has been revised The specific aren't important, but this does present additional surface area for bugs. **Performance Improvements** Pydantic v2 is a full rewrite with a rust backend. This offers a substantial performance improvement (pydantic claims 5x to 50x depending on the task). We'll notice this the most during serialization and deserialization of sessions/graphs, which happens very very often - a couple times per node. I haven't done any benchmarks, but anecdotally, graph execution is much faster. Also, very larges graphs - like with massive iterators - are much, much faster. | 2 年前 | |
fix(model cache): evict records at shutdown() instead of only releasing shared weights (#9494) * fix(model cache): release shared weights when a cache goes away Nothing released a cache's SharedCpuWeightsStore references except _delete_cache_entry(): shutdown() left every resident record's refcount held, and a cache dropped without shutdown() (test teardown; any future wiring that rebuilds caches at runtime) stranded the canonical tensors and their accounting forever. Today's production wiring tears the store down together with its caches, so the live exposure is cross-test pollution of the process-global store and RAM pinned past ModelManagerService.stop() — but the refcount invariant ('every acquire is paired with exactly one release') was simply not upheld, and this makes it self-healing before any wiring change turns it into a real peer-accounting bug. Two mechanisms, for the two ways a cache goes away: - shutdown() now releases its resident records' shared references synchronously — it runs in a normal thread context, so the direct (locking) release is safe there, and teardown does not depend on a later store operation happening. - Each wrapper registers a weakref.finalize fallback for the dropped-without-shutdown case. The finalizer runs in GC context, where taking the store's non-reentrant lock could self-deadlock (a collection can fire inside acquire()'s critical section on the same thread — the rule ModelCache.release_first_use_grace documents), so it only ENQUEUES into a SimpleQueue; every public store method drains the queue under the lock. The finalizer is registered inside the acquire's try (a registration failure must release too), its args carry the key and canonical dict rather than the wrapper (finalize holds args strongly — referencing self would make the wrapper immortal), and release_shared_weights() detaches it before releasing synchronously so eviction-then-collection releases exactly once. The state-dict identity keeps releases correct across invalidate()'s retired entries. RamBudget.total_in_use() now documents why its store read must stay outside the budget lock: the drain allocates under the store lock, so GC can run _on_cache_collected (store→budget) there, and a budget→store order anywhere would complete the deadlock cycle. Six regression tests, verified to fail before the fix, covering: shutdown releases synchronously with an empty queue; collection returns refcount/bytes/budget to zero; the collection-time release is enqueue-only (never applied inline by GC); eviction + collection release exactly once across two caches; a retired (invalidated) entry is freed by a collected holder; and the partial-load wrapper behaves like the full-load one. One existing test relied on an abandoned wrapper leaking its reference and now binds it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(model cache): evict records at shutdown() instead of only releasing shared weights shutdown() released the resident records' shared-store references while retaining the records themselves, so the accounting stopped describing reality: - The store (and RamBudget) reported zero for bytes whose tensors the retained wrappers still held. - A post-shutdown load of the same key on a peer cache registered a duplicate canonical alongside the still-resident released copy. - A post-shutdown eviction of a released record (put() after shutdown() is reachable: Invoker.stop() stops the model manager before the session processor) read uses_shared_weights as already-False and debited the non-shared budget for bytes that were admitted as shared. shutdown() now routes idle records through _delete_cache_entry(), which releases shared ownership and budget accounting together, exactly once. Records still in use — locked by an in-flight generation or inside the put()->lock() admission window — keep their references and are marked stale; unlock() evicts them through the existing stale path when the generation lets go, so the accounting stays truthful at every point. All five regression tests verified to fail against the previous shutdown() behavior. Follow-on to #9403, addressing JPPhoto's review comment there. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(model cache): match records by identity in stale eviction and _delete_cache_entry Surfaced by adversarial review of the shutdown() change: a stale-marked record can be detached while still locked (the VRAM-move error paths call _delete_cache_entry on a locked record) and its key re-admitted before the record's last unlock(). The stale-eviction path matched by key only, so it popped the NEW record — detaching it from the cache and all accounting — and, the old record's shared release having already happened, read uses_shared_weights as False and debited the non-shared budget for bytes that were admitted as shared. The hazard predates the shutdown() change (drop_model() sets the same flag), but shutdown() now arms stale marks at every server stop that overlaps in-flight work, so close it here: _delete_cache_entry() and unlock()'s stale eviction act only when the record passed in IS the record currently held under its key; a delete of a detached record is a full no-op. Regression test verified to fail against the key-only matching. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(model cache): track get()->lock() holders through shutdown and abandonment Two defects found in review of the shutdown eviction change (JPPhoto, 2026-08-13): 1. shutdown() racing the gap between get() and the LoadedModel's first lock evicted the warm record out from under its holder: the holder locked a detached record whose shared-store ownership had just been released, so a peer's reload of the same key minted a duplicate canonical copy while the budget counted one. 2. A record retained by the shutdown sweep for a never-locked holder could never be evicted if that holder was simply dropped: the abandonment finalizer's deferred work was discarded post-shutdown (and the worker was stopped), pinning the record, its shared-store refcount and its budget bytes for the life of the process. The fix tracks every wrapper's get()->lock() window with a per-record hold count (CacheRecord.first_use_holds), armed in LoadedModelWithoutConfig's constructor and released exactly once per wrapper — on its first lock, or by its weakref finalizer if it is dropped un-entered. Held records are treated like locked ones by every eviction path (shutdown, budget reconcile, peer-requested eviction, make_room, drop_model, unlock's stale eviction); stale-marked records whose last holder is abandoned are evicted by the deferred worker, which now outlives shutdown() for exactly that purpose (it already exits via the cache-collection finalizer). Holds are only granted while a worker is alive to carry the finalizer's release, and a worker death zeroes surviving holds at the next start so no record can stay shielded with nothing left to unshield it. Admissions landing after shutdown() are marked stale at birth so their final release evicts them too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(model cache): epoch-guard hold releases and recover stranded holds at shutdown Hardening from adversarial review of the first-use-hold mechanism: - Hold releases (the wrapper's first-lock release and the abandonment finalizer's deferred release) now quote the epoch the hold was armed under, and dead-worker recovery bumps the record's epoch when it zeroes stranded holds. Without this, a surviving wrapper's late release — or a release enqueued before the worker died and drained after the restart — would decrement a fresh hold armed by a different wrapper under the healthy replacement worker, silently unshielding that wrapper's window. - shutdown() now runs the dead-worker hold recovery itself (and clears the put()-grace flags in the same situation): a hold whose abandonment release was dropped by the dead-thread dispatch check has no other releaser, and after shutdown no put() is guaranteed to run the usual next-start recovery — the sweep would stale-retain the record, its shared-store refcount and its budget bytes for the life of the process. - register_first_use_hold() declines to arm on a record that is no longer the occupant under its key: an eviction already won the race against the wrapper's construction and a hold on a detached record shields nothing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(model cache): withhold the post-shutdown grace and recover from the worker's own death Two follow-ups from review. put() after shutdown() no longer arms the post-admission grace. That flag's backstop releaser is the sweep at the top of the next put(), and after shutdown no further put() is guaranteed: a load cancelled between put() and the LoadedModel's construction leaves no wrapper (hence no finalizer either), so an armed flag would stand for the life of the process, hiding the record from every asynchronous eviction path while its bytes stayed charged to the shared budget. Withholding it costs only the shield -- the record stays stale at birth, so its eventual release still evicts it, and a loader that does come back gets the ordinary first_use_holds shield. The deferred worker now runs stranded-shield recovery from inside its own dying frame. Previously recovery depended on something else happening first -- the next admission, or shutdown() -- and neither is guaranteed when the worker dies *after* shutdown()'s liveness check: the records the shutdown sweep retained for a live holder were left shielded by holds nothing could release. The recovery is scoped by thread identity (a replacement worker's shields are its own) and retires the worker slot before sweeping, so a concurrent admission cannot arm a shield the recovery is about to zero. It also drains the queue the dead worker left behind, whose _AbandonedHolderRelease items pin their models' CPU weights. _ensure_deferred_worker and shutdown() now share the same recovery, which also lifts orphaned admission graces and evicts whatever that leaves unshielded. Three tests, each reverted-and-confirmed-failing against the code it guards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GjAw9hpJe8d1GyvpdKJFGw * fix(model cache): narrow the dead-worker recovery and stop it pinning records Adversarial review of the previous commit found three problems with the shared recovery it introduced. The recovery cleared the put()-set admission grace unconditionally. On a live cache that is a new failure mode, not a fix: the dying worker is is_alive() for as long as it unwinds, so a cold load landing in that window starts no replacement worker and is admitted with the ordinary grace, which the recovery then zeroed while the loader was still between put() and get() -- a reconcile could evict the record and the loader's get() would raise IndexError. The grace only actually loses a releaser once the cache is shut down (its backstop is the next put()'s sweep, not the worker), so it is now lifted only then. The recovery also evicted stale-unshielded records from _ensure_deferred_worker, which register_first_use_hold calls before arming -- so a second wrapper's construction could detach the very record it was about to shield, releasing shared-store ownership while live wrappers still held the tensors. That is the accounting lie shutdown() itself refuses to make. The eviction moved to _evict_stale_unshielded_entries, called only from the dying worker and only on a shut-down cache, where nothing else can ever run it; it now also collects and empties the device cache the way the other abandonment path does. Keeping shutdown()'s call to pure field assignments restores its old property that the branch cannot raise before the resident-record sweep. The queue drain the previous commit added did not close the pin it targeted: _dispatch_deferred's liveness gate is unsynchronized, so a finalizer that read the worker slot just before it was retired still enqueues after the drain. The drain is gone; _AbandonedHolderRelease now holds its record weakly instead, so a stranded item pins nothing, and the worker clears the strong reference it resolves before parking on the next get(). Also moves _reconcile_budget_if_pending's lock acquisition adjacent to its try: a BaseException between the two leaked the cache RLock to an unwinding thread, blocking every other thread for the life of the process. Five tests, each reverted-and-confirmed-failing against the code it guards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GjAw9hpJe8d1GyvpdKJFGw * fix(model cache): key the dead-worker backstops on worker liveness, not the slot A second adversarial pass found that retiring the worker slot from inside the dying worker silently disabled both remaining recovery sites, which gated on "a dead thread still occupying the slot". The dying recovery deliberately leaves a live cache's admission grace standing -- the next put()'s sweep is still its backstop -- and hands the lift to shutdown(); with the slot already empty, shutdown() skipped it and stale-retained the record, its shared-store reference and its budget bytes for the life of the process. Both gates now key on "no live worker": shutdown() lifts when the slot is empty or dead, and the worker start recovers unconditionally (it has already returned if a worker is alive). That also makes a failed recovery retryable, which matters because the recovery was not exception-safe and had already retired the slot by the time it could raise. _clear_stranded_first_use_holds now unshields every record before reporting any of them -- a logging handler that raises is one of the ways the worker dies in the first place, and logging inline let that same handler abort the sweep partway -- and the post-eviction gc/empty_cache housekeeping, which the codebase already documents can raise from a sick CUDA context, no longer takes the eviction down with it. Also corrects two overstated claims in the weakref rationale: a stranded queue item can be drained later by a replacement worker (the queue is per-cache, not per-worker), and the hold decrement in _release_abandoned_holder runs before the identity check -- it is inert on a detached record for a different reason, which the docstring now gives. Moving _reconcile_budget_if_pending's acquire adjacent to its try narrows the RLock-leak window rather than closing it; said so. Three tests, each reverted-and-confirmed-failing against the code it guards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GjAw9hpJe8d1GyvpdKJFGw * fix(model cache): claim the first-use window at the lookup, and let a cancelled admission release itself Two findings from review. The first-use shield was armed by LoadedModel's constructor, leaving the whole stretch between the cache lookup and that constructor unshielded -- and that stretch is not a couple of instructions: the configured loader retrieves its record inside _load_and_cache and then does the shared-store shell registration and two returns before load_model wraps it. A shutdown sweep or a peer's reconcile landing there detached the record its holder was about to lock, releasing shared-store ownership while the tensors lived on, so a peer's reload minted a duplicate canonical the budget counted once. ModelCache.get_with_first_use_claim() now arms the hold in the same lock acquisition as the lookup and hands back a FirstUseClaim that owns it: the wrapper adopts the claim and releases it at its first lock, and a claim dropped without ever being adopted -- the load raised before a wrapper existed -- releases the hold by dying. shutdown() stale-retained a record carrying only the put()-set admission grace. That grace's three releasers are the loader's own get()->lock(), the abandonment finalizer of a wrapper built from the record, and the sweep at the top of the next put(); a load cancelled between its put() and its retrieval has neither of the first two, and after shutdown no further put() is guaranteed to run the third, so the record, its shared-store reference and its budget charge stood until the cache object was collected. put(claim_admission=True) now hands the loader a claim over that window too, so such a load releases its admission by dying and the shutdown sweep finds an ordinary idle record. Retiring the grace at shutdown instead -- the obvious shortcut, and what the first two drafts of this commit did -- is not safe. The flag is unowned, so a standing grace does not mean nobody is working on the record: it is equally the state of a load still between its put() and its retrieval, and of a live un-entered wrapper whose hold a worker death zeroed. Both were evicted out from under their holder, with the duplicate-canonical accounting lie and an IndexError from a retrieval that no longer found its own model. For the same reason the admission window is not shielded by either flag but by a weak reference to the claim (CacheRecord.admission_claim_ref): nothing has to release it, so neither a worker death (which zeroes holds) nor another holder's abandonment (which clears the grace) can make a running load look finished. _recover_stranded_shields retires it once the cache is shut down, where the eviction its expiry should trigger would otherwise travel through a dead worker -- the same trade that method already makes for holds. The claim is armed only after put() has committed its accounting, so a failure to allocate it cannot leave a resident, store-owning record the budget never counted, and both hand-back guards release the hold when the object that was to carry its release cannot be built. Nine tests, each reverted-and-confirmed-failing against the code it guards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FCitU6EFcp76AaauNfaWTz * fix(model cache): refuse post-shutdown prefetch admissions, and keep abandonment releases Two records could survive a shut-down cache with nothing left to retire them. A prefetch=True admission after shutdown() was marked stale and inserted anyway. prefetch is the promise that no loader will come back for the record, so there is no get() -> lock() -> unlock() to run the stale eviction, no wrapper whose finalizer could carry an abandonment release, and no claim whose expiry could stand in for either. What is left are the paths that may or may not run -- another admission's make_room, a budget reconcile, a peer's eviction request -- and after shutdown none of them is guaranteed to come. put() now refuses such an admission, the same standard the post-admission grace is already withheld under. It costs only a reload: the sole caller takes the submodel it asked for from the pipeline object, not from the cache. The refusal sits after _ensure_deferred_worker() (a post-shutdown prefetch must still revive the worker that carries the retained records' abandonment releases), after the stale-grace sweep (on a shut-down cache with a LIVE worker that sweep is the only backstop a stale grace has left, since shutdown() runs the recovery only when no worker is alive), and before _make_room_internal (nothing resident should be evicted to house a model this call is about to refuse). _dispatch_deferred dropped every item while no worker was running, which included _AbandonedHolderRelease. Its holder is already gone -- finalizers fire once -- so no lock, no unlock and no second finalizer is coming, and that item is the only thing left that can retire the record. Dropping it stranded a record the shutdown sweep had retained, resident and charged, for the life of the process; no later sweep can repair that, because once dead-worker recovery zeroes the hold, a record whose holder is gone is indistinguishable from one a live wrapper is still holding, where retention is required. Abandonment releases are now kept and drained by whichever worker runs next; reconciles are still dropped, since cached_model_keys() can request one on every call and the next cache operation's release hook re-runs it anyway. Evicting from _ensure_deferred_worker() is deliberately NOT the fix: it runs _recover_stranded_shields() immediately before, so at that instant a record a live wrapper still holds looks unshielded, and evicting it would release shared-store ownership while the tensors live on. Four tests, each checked for sensitivity by reverting the line it guards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015n81CKSi7bUA217L2E9SwN * fix(model cache): bound the abandonment queue, own worker-less admissions, keep live claims through recovery JPPhoto's round-6 findings on the shutdown accounting, all three confirmed: - A grace-only abandonment enqueued one kept item per dropped wrapper while no worker could be started, and nothing short of a lock() or another put() ever cleared the grace that kept them coming, so a warm get/drop loop grew the queue without bound. release_first_use_grace now clears the grace itself, lock-free (the flag is monotonic), queues an eviction only for a stale record and only once per record (CacheRecord.abandonment_release_pending, re-opened by the worker the moment it dequeues the item), and still wakes the worker for a pending budget reconcile the drained item used to run. - A post-shutdown put(claim_admission=True) with no startable worker got no claim, so nothing owned the record once its loader died and no later admission swept it. _claim_first_use now mints a hold-less FirstUseClaim (hold_epoch=None) whenever the record is still the occupant, so the admission stays owned through CacheRecord.admission_claim_ref and its finalizer still queues the eviction a stale record owes; and put() runs _evict_stale_unshielded_entries() when the cache is shut down and no worker is alive after its revival attempt, the same terminal sweep the dying worker runs, placed after the grace sweep and before the prefetch refusal. - _recover_stranded_shields retired a live admission claim on a shut-down cache, so a worker death followed by shutdown() evicted a record whose loader was between put() and get(), and that loader's retrieval raised IndexError. A live claim now survives every recovery; the eviction owed once it dies travels through the kept queue item or the next admission's sweep. Two further defects surfaced by the adversarial passes over the fix: shutdown() now marks a record stale BEFORE consulting its shield, so a hold-less abandonment racing the sweep either sees the mark and queues the eviction or has already cleared its shield when the sweep looks; and the coalescing gate is opened at the dequeue site rather than in the handler, so a raise in the handler, a worker death inside it, or a fork cannot leave it closed with nothing queued behind it. Ten new tests, two rewritten; each production change reverted individually and confirmed to fail only the tests that guard it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011JbML8Y8mHdXH72bcUry9o * fix(model cache): refuse worker-less post-shutdown admissions JPPhoto's round-7 blocker, the residual I disclosed rather than closed last round: a claimed admission into a shut-down cache with no startable worker has no releaser (no worker to drain the eviction its dropped claim's finalizer queues) and no guaranteed future cache operation to stand in for one, so a load that dies before retrieving it pins the record, its shared-store reference and its budget bytes for the life of the cache object. put() now refuses every admission -- claimed, plain, and prefetch -- once the cache is shut down and no worker is alive after its revival attempt, the same standard the post-shutdown prefetch was already refused under, generalized. Nothing that could be stranded is admitted. The refusal is narrowly scoped: a live cache still admits worker-less (a future op cleans up), and a normal shutdown keeps its worker alive so the graceful retain-and-reclaim path is unchanged -- only thread exhaustion reaches the refusal, where a load racing shutdown is failing regardless (its retrieval raises IndexError, as a refused prefetch's does; both loaders already handle a None put()). The terminal sweep (_evict_stale_unshielded_entries) still runs before the refusal returns, so a record already orphaned by a worker death is reclaimed even though this admission -- possibly the last cache operation -- is refused. Three tests whose orphan came from the now-refused admission are removed; their mechanism coverage survives elsewhere. Two added: the refusal with its clean loader-style IndexError, and the terminal-sweep-still-runs case. Both new production lines mutated and confirmed to fail only their guarding tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011JbML8Y8mHdXH72bcUry9o * fix(model cache): clean shutdown-retained records --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Jonathan <34005131+JPPhoto@users.noreply.github.com> Co-authored-by: JPPhoto <jpollack@jpollackphoto.com> | 1 小时前 | |
Remove dependency on flux config files | 1 年前 | |
fix(ui): stop range-based fetching hooks from spinning in a render loop (#9439) * fix(ui): stop range-based fetching hooks from spinning in a render loop `fetchItems` cleared the accumulated ranges with `setPendingRanges([])`, and `pendingRanges` is a dependency of the effect that calls `fetchItems`. A fresh `[]` is a new identity every time, so the effect re-ran, re-armed the 500ms throttle, and cleared again — a self-sustaining render loop that ran as fast as the throttle allowed, with no user input, for as long as the gallery grid was mounted. Clear with the shared stable `EMPTY_ARRAY` reference instead, so React bails out rather than re-running the effect. The queue variant returned early — before clearing — when nothing was uncached, which happened to prevent the loop while everything was cached, at the cost of letting ranges accumulate for the lifetime of the list and growing the scan on every pass. It now clears on both paths, with the stable reference doing the work of stopping the loop. Retry on failure explicitly, because the loop was doing it accidentally. These bulk fetches are the only fetcher for their rows: `ImageAtPosition` and `QueueItemAtPosition` both consume the cache with `skip: isUninitialized`, so a row whose DTO never arrived does not fetch for itself, and images have no retry affordance. Without this, a transient failure would leave placeholders until the user happened to scroll, where before the loop re-tried until it succeeded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(ui): regression tests for the range-based fetching render loop Render both hooks with React act + fake timers in a happy-dom environment (scoped per-file via a @vitest-environment docblock; happy-dom is the only new dev dependency) and mock only the thin API-endpoint modules, so the tests exercise the real state/effect/throttle cycle the fix changed. Covered per hook: - a reported range fetches its uncached items once, then renders and fetches both go quiet (the pre-fix loop re-rendered every throttle window forever, and in the gallery hook ran from mount even with nothing to fetch) - items that never land in the cache (deleted image, multiuser ownership filter) are not re-requested indefinitely — bounded, then quiet, where the pre-fix loop was a permanent one-request-per-window stream - a failed bulk fetch is retried until it succeeds, then goes quiet — the explicit replacement for the retry the loop provided accidentally - every range reported within a throttle window is fetched, not just the last (the pendingRanges accumulation onRangeChanged exists for) - handled ranges are dropped, not accumulated: an item evicted from a long-handled range is not re-requested by later passes (the queue hook's pre-fix early return without clearing regressed exactly this) - new ranges after settling still fetch, and enabled=false fetches nothing The time-advance helper steps in small increments with an act flush per step; a single long advance would defer effect re-runs to the end of the act scope and break the very feedback cycle (state update -> effect -> throttle -> fetch) the suite exists to detect. Mutation-verified: reverting the EMPTY_ARRAY clears, restoring the queue hook's early return, dropping onRangeChanged's accumulation, or neutering the retry catch each makes at least one test fail; all pass with the fix in place. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ui): bound the range-fetch retry with backoff and coalesced ranges Review feedback on the retry added in this PR: restoring the failed ranges immediately meant a sustained backend outage produced a request every throttle window forever, the restored state grew by a duplicate range per cycle, and `prev.length > 0 ? prev : ranges` dropped a failed range whenever another had been reported in the meantime. Replace the immediate restore with a shared useBoundedRangeRetry hook: - Exponential backoff between retries (1s, 2s, 4s, 8s, capped at 16s), giving up after 5 consecutive scheduled retries, so a sustained failure terminates instead of storming a backend that is trying to come back up. - Failed ranges accumulate as a coalesced (sorted, disjoint) union, and the restore merges them into whatever is pending instead of choosing one side, so nothing is dropped and nothing grows without bound. - A new range report resets the retry budget: fresh user input revives a list that gave up, and rows still in view are re-reported by virtuoso when the user scrolls back anyway. Tests: negative-path coverage for both hooks (sustained failure terminates; scrolling revives a given-up list; a range that failed mid-scroll is recovered) plus unit tests for coalesceRanges. Mutation-verified: removing the backoff/cap, the budget reset, the merge-on-restore, or the retry itself each makes at least one test fail; all 30 pass with the change in place. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ui): heal an abandoned range fetch on reconnect, and bound the retry's lifetime Round-2 review findings on the bounded range-fetch retry. - Giving up was permanent. The budget ends ~31s after the first failure, but an InvokeAI restart routinely takes longer, and for an idle user nothing re-arms it: `imageNames` keeps its identity through a reconnect refetch, `enabled` (`!isLoading`) does not toggle on a refetch, and in production `socketConnected` only invalidates `FetchOnReconnect` when the queue status changed. Ranges abandoned by an exhausted budget are now parked as a coalesced union instead of dropped, and restored on the next signal that the backend is answering: a socket reconnect, a successful fetch, or a fresh range report. - A fetch that rejected after unmount armed a backoff timer no cleanup could reach. The retry state now tracks mount status and drops late failures. - The `!enabled` guard returned before the clear — the same accumulate-forever pattern this PR fixes on the cached path. Both hooks now clear on that path. - `restoreRanges` is read through a ref, so an unstable callback can no longer churn `onFetchFailure` and the fetch effect behind it. Tests: reconnect healing, a post-unmount rejection arming no timer, and the disabled-window accumulation case in both hook suites, plus the queue suite's missing mount-time no-loop test. Each is mutation-verified against the fix it covers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QWxRMfb5wDBi6isQ6XKrgE * fix(ui): stop losing restored ranges to the throttle, and floor the reconnect re-arm Adversarial review of the previous commit found two real defects. Restored ranges could be lost permanently. `restoreRanges` dispatches a functional `setPendingRanges`, but the fetch pass ended with an absolute `setPendingRanges(EMPTY_ARRAY)`. A backoff timer and the throttle's trailing edge can expire in the same event-loop turn, so both land in one React batch: the absolute update runs last, the final state equals the base, React bails out of the re-render, and the ranges are gone with nothing left to re-report them. Reproduced deterministically in both hooks (fail a range, scroll elsewhere 600-1000ms later, recover: the first range is never fetched again — grey rows until the user scrolls back). The clear now only fires when `pendingRanges` is still the array that pass consumed, so it is a no-op once the state has moved on. The existing scroll-recovery test was pinned to a delay that happened to miss this window; it now sweeps 500-1250ms and fails at three of five without the fix. The reconnect signal made the bounded retry unbounded. `attempts` was zeroed on every `$isConnected` transition, even with nothing parked, so a socket that keeps completing a handshake while REST stays broken (crash-looping container, uvicorn accepting connections before startup finishes, a proxy splitting websocket and REST across replicas) pinned the backoff at its shortest delay: 300 requests over five minutes of 5s flapping, against a design intent of 12. The re-arm is now floored at one per 60s and only fires when there is something parked to heal — 70 requests in the same scenario. Also: the latest-callback ref moved to a layout effect so a restore firing before the passive flush sees the intended closure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QWxRMfb5wDBi6isQ6XKrgE * test(ui): pin the scroll/success restore of parked ranges; drop a stray vitest artifact Review round 3, both findings. `useBoundedRangeRetry` restores parked ranges on three signals — a socket reconnect, a later successful fetch, and a fresh range report — but only the reconnect was pinned. `resumes retrying after giving up when the user scrolls` re-reports the same range, which `lastRange` re-fetches whether or not the parked set was restored, so deleting the restore block from `resetRetryBudget` left every test green. The new test parks a range under sustained failure, has the backend answer again with no socket transition (a transient proxy 502, where the websocket never drops), then scrolls to a disjoint range, and asserts both the parked and the new range are fetched. Mutation-verified: removing the restore fails it in both suites. Behaviour is unchanged; this is coverage only. Also drops `node_modules/.vite/vitest/.../results.json`, committed by accident — the root .gitignore had no `node_modules` entry (only the web app's does), so a repo-root `node_modules/` was untracked but not ignored. Added it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FCitU6EFcp76AaauNfaWTz * test(ui): pin the parked-set clear and both coalescing call sites Round-4 review findings, both coverage-only: - 'empties the parked set when it heals' passed with the clear in takeAbandonedRanges deleted: with no second heal signal, a re-restore installs the same array reference while pendingRanges holds EMPTY_ARRAY, React bails out, and the fetch count stays flat either way. Both suites now scroll to a disjoint range after the heal and assert only the new range is requested — a parked set that outlived its restore rides along and fails the assertion. - Neither coalesceRanges call site was pinned: replacing either accumulation with a plain concat left every suite green, so the bounded-state property rested on the helper's unit tests alone. Two hook-level tests now observe what is actually handed to restoreRanges: failures merged while a retry is scheduled arrive as one coalesced union, and repeated post-exhaustion failures park as one. All three mutants now die by exactly the intended tests (2/1/1 failures). Full suite: 171 files, 2275 tests, five lints clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WUz6V3apfDE3hCrhGfMDpt --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Alexander Eichhorn <alex@eichhorn.dev> | 2 天前 | |
Add OKLab and Oklch image utilities and nodes, refactor color space nodes (#8999) * Add OKLab and Oklch image utilities and nodes * refactor: unify oklab color conversions * refactor: unify shared color conversions * chore: typegen * chore: typegen --------- Co-authored-by: Lincoln Stein <lincoln.stein@gmail.com> | 3 个月前 | |
Lstein/chore/6.14.0 post1 bump (#9543) * chore(release): bump version * docs: update WhatsNew * chore(release): bump version * chore(frontend): lint prettier * bump main version | 11 天前 | |
Various fixes 1) Downgrade numpy to avoid dependency conflict with numba 2) Move all non ldm/invoke files into `invokeai`. This includes assets, backend, frontend, and configs. 3) Fix up way that the backend finds the frontend and the generator finds the NSFW caution.png icon. | 3 年前 |
| 文件 | 最后提交记录 | 最后更新时间 |
|---|---|---|
| 1 小时前 | ||
| 2 年前 | ||
| 1 小时前 | ||
| 1 年前 | ||
| 2 天前 | ||
| 3 个月前 | ||
| 11 天前 | ||
| 3 年前 |