Make Valkey GLIDE script-handle cleanup exception-safe

[lan17]

Priority

P3 — Low — lifecycle hardening for rare native construction and cleanup failures.

v0.14.0 grooming verification (2026-08-01)

Verified on main@34bd022: five native scripts are still constructed without rollback, and disposal still stops after the first throwing release(). Ready batch-invalidation PR #114 edits this adapter but leaves both lifecycle failure modes intact. Keep P3 and implement after PR #114 is merged or rejected so the final handle set is covered once.

Current baseline

At v0.11.0 / e06d833ba245706a499d66056fdad15dc1210b68, the Valkey GLIDE adapter owns five native Script handles and exposes synchronous dispose() to release them after application work drains. Normal disposal and the active-invocation guard are tested. Partial construction and throwing release() paths are not exception-safe.

Problem

The factory constructs all five handles inside one object literal. If construction of the second through fifth handle throws, previously created handles are unreachable because the adapter is never returned, so their native registrations cannot be released.

During disposal, the adapter sets disposed = true and then releases handles sequentially. If one release() throws, later handles are not attempted and a later dispose() returns immediately, leaving cleanup permanently incomplete.

The public structural ValkeyGlideScriptHandle contract does not promise that either construction or release() is non-throwing. Native-resource boundaries should remain cleanup-safe even when an exceptional failure occurs.

Pinned evidence

• The public handle contract exposes explicit release ownership:

  DialCache/src/valkey-glide.ts

  Lines 17 to 20 in e06d833

  	export interface ValkeyGlideScriptHandle {
  	/** Release the native GLIDE script registration. */
  	release(): void;
  	}

• Public TSDoc says the returned adapter owns its Script handles:

  DialCache/src/valkey-glide.ts

  Lines 55 to 63 in e06d833

  	/**
  	* Wrap a caller-owned GLIDE connection. The returned adapter owns only its
  	* Script handles and preserves the connection's `requestTimeout`. Pass the
  	* same GLIDE module namespace used to create the client so native Script
  	* handles are registered with that client's runtime. Callers dispose the
  	* handles after draining work, then close GLIDE. A request timeout bounds
  	* client waiting but is not server-side command cancellation. GLIDE's current
  	* script API has no per-invocation signal, so DialCache's core read deadline
  	* may return before this adapter's invocation settles.

• Five handles are constructed without rollback:

  DialCache/src/valkey-glide.ts

  Lines 65 to 75 in e06d833

  	export function createValkeyGlideDialCacheClient<TScript extends ValkeyGlideScriptHandle, TDecoder>(
  	client: ValkeyGlideScriptingClient<TScript, TDecoder>,
  	glide: ValkeyGlideRuntime<TScript, TDecoder>,
  	): ValkeyGlideDialCacheClient {
  	const scripts: DialCacheGlideScripts<TScript> = {
  	read: new glide.Script(READ_CACHE_SCRIPT),
  	readTracked: new glide.Script(READ_TRACKED_CACHE_SCRIPT),
  	write: new glide.Script(WRITE_CACHE_SCRIPT),
  	writeTracked: new glide.Script(WRITE_TRACKED_CACHE_SCRIPT),
  	invalidate: new glide.Script(INVALIDATE_CACHE_SCRIPT),
  	};

• Disposal stops at the first throwing release after marking the adapter disposed:

  DialCache/src/valkey-glide.ts

  Lines 128 to 139 in e06d833

  	dispose() {
  	if (disposed) {
  	return;
  	}
  	if (activeInvocations > 0) {
  	throw new Error("Cannot dispose Valkey GLIDE DialCache client while operations are in flight");
  	}
  	disposed = true;
  	for (const script of Object.values(scripts)) {
  	script.release();
  	}
  	},

• Existing tests cover successful idempotent disposal and in-flight rejection only:

  DialCache/test/valkey-glide.test.ts

  Lines 213 to 247 in e06d833

  	it("releases every script exactly once and rejects later operations", async () => {
  	const client = fakeClient();
  	const adapter = createValkeyGlideDialCacheClient(client, mockGlide);
  	adapter.dispose();
  	adapter.dispose();
  	expect(scriptInstances).toHaveLength(5);
  	for (const script of scriptInstances) {
  	expect(script.release).toHaveBeenCalledTimes(1);
  	}
  	await expect(adapter.read({ valueKey: "disposed" })).rejects.toThrow("Valkey GLIDE DialCache client is disposed");
  	expect(client.invokeScript).not.toHaveBeenCalled();
  	});
  	it("does not release scripts while an invocation is in flight", async () => {
  	let resolveRead: ((value: Buffer) => void) | undefined;
  	const client = fakeClient();
  	client.invokeScript.mockImplementationOnce(
  	async () => await new Promise<Buffer>((resolve) => {
  	resolveRead = resolve;
  	}),
  	);
  	const adapter = createValkeyGlideDialCacheClient(client, mockGlide);
  	const read = adapter.read({ valueKey: "in-flight" });
  	expect(() => adapter.dispose()).toThrow(
  	"Cannot dispose Valkey GLIDE DialCache client while operations are in flight",
  	);
  	expect(scriptInstances.every((script) => script.release.mock.calls.length === 0)).toBe(true);
  	resolveRead?.(Buffer.from([0, ...Buffer.from("done")]));
  	await expect(read).resolves.toBe("done");
  	adapter.dispose();
  	expect(scriptInstances.every((script) => script.release.mock.calls.length === 1)).toBe(true);

Simplest scope

• Construct handles incrementally while tracking every successfully created handle.
• If later construction fails, attempt to release all earlier handles before propagating the construction failure.
• Centralize handle release in one small internal helper.
• During dispose(), attempt every handle release even if one fails.
• After all attempts, preserve and surface the failure; do not let one handle prevent attempts for the others.
• Keep the adapter unusable after disposal begins.
• Preserve normal idempotence and the current “cannot dispose while operations are in flight” guard.
• Do not add async disposal, automatic draining, Symbol.dispose, another public lifecycle method, or ownership of the caller's GLIDE connection.

Acceptance criteria

• If construction fails at positions two through five, every previously constructed handle has release() attempted exactly once.
• Cleanup failure during construction does not prevent attempts for the remaining already-created handles.
• The original construction failure remains observable; cleanup failures are retained without silently replacing or dropping all context.
• If any release() throws during normal disposal, every handle still has release attempted, the cleanup failure is surfaced, and the adapter rejects all later operations.
• Successful disposal remains idempotent and releases every handle exactly once.
• Calling dispose() while an invocation is active still releases nothing and throws the existing focused error.
• The adapter never closes or disposes the caller-owned GLIDE connection.
• No public type, Redis script, key, payload, or invocation behavior changes.
• Focused tests cover each constructor-failure position, each release-failure position, successful idempotence, and active-operation protection.
• Typecheck, unit tests, build/declarations, packed GLIDE consumers, and Redis integration tests remain green.

Compatibility

No normal-path API change is intended. The only observable difference is more complete cleanup and better error preservation when Script construction or release already fails.

Related issues

• Require caller-owned Redis clients and remove the lazy factory #20 — caller-owned Redis resources and adapter lifecycle.
• Validate exact Lua reply domains in bundled Redis adapters #30 — bundled-adapter boundary validation.