Skip to content

models: backend registry replacement orphans the old backend — no dispose lifecycle on ModelBackend #1758

Description

@heskew

Problem

The backend registry (resources/models/backendRegistry.ts) replaces registrations with a plain Map.set() — re-registering a logical name (or clearRegistry()) drops the old ModelBackend instance without any notification. For the built-in HTTP backends that's just garbage collection. For in-process backends — the case #1325/#1471 explicitly support — the instance can own real native resources: a config-selectable GGUF embedding backend holds llama.cpp model weights and a decode context (heskew/harper-fabric-embeddings#3). Silent replacement leaks them for the life of the process, and process shutdown never notifies backends either.

Severity framing (honest): today bootstrapModels runs from a single boot-time call site (componentLoader.ts:373, root config only), so replacement effectively doesn't happen in production yet — the leak is latent. But the contract gap is real for (a) any future config hot-reload of the models: block, (b) apps that call registerBackend() more than once per process (component reload paths), and (c) graceful shutdown of backends with connections/handles.

Sketch

Optional lifecycle method on the contract:

interface ModelBackend {
	// ...
	/** Release resources. Called by the registry when this instance is replaced or cleared. */
	dispose?(): Promise<void> | void;
}

Registry behavior:

  • setEmbedding/setGenerative (and therefore registerBackend): when the slot already holds a different instance, call the old instance's dispose() after the swap — fire-and-forget with error logging, so a failing dispose can't break registration.
  • clearRegistry(): dispose all, then clear.
  • Optionally: a shutdown hook wired to the same place component scopes get closed.

The subtlety worth deciding up front: one backend instance can legitimately be registered under multiple logical names (aliases pointing at one engine). Name-slot replacement must not dispose an instance still referenced by another name — the registry needs an identity check across both kind-maps before disposing (or we document one-name-per-instance as a contract). This is the main reason to design this in core rather than have each backend hand-roll it.

defineBackend passes dispose through from the spec.

Related: #1325 (registerBackend/defineBackend), #1471 (config-selectable module backends), #1235 (models subsystem audit).

🤖 Generated with Claude Code

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions