fix: bound resumable teardown, image concurrency and delivery leases

TR-RR-06. dispose() now bounds its drain with a cleanupDeadlineMs from policy
and returns the result, so a non-cooperative mutation lock or provider can no
longer make teardown unbounded and an unproved drain is reported as still
CLOSING instead of closed over. The checkpoint store stays open in that case,
because something can still write to it. An abort is admitted physical work
like an upload, so it joins the tracked set rather than being stepped over.

TR-RR-07. The verification slot belongs to the raw verifier, not the wrapper.
Releasing it when the caller's wait expired let an abandoned verification keep
running while a new one was admitted, so repeated aborts produced more
concurrent physical work than the configured cap allows. The slot is now
released only once the raw tasks settle.

TR-RR-04. A presigned byte source owns a fetch reader and a capability lease and
its port requires close(); the delivery consumer never called it. The closeable
subtype is lost in the FileByteSource projection, so a holder keeps it from the
moment the lease exists and the outermost finally closes it exactly once — on
success, validation failure, writer failure and abort alike.

check:adapter-inventory now also fails if the shared abortable-operation
primitive has no production importers. It was safe to add only once the
presigned subsystems actually migrated onto it; a gate that fails CI for a
documented, unfixed defect reports the wrong thing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
DongHyeonka
2026-08-14 17:22:45 +09:00
co-authored by Claude Opus 5
parent 46e067e555
commit 5a76f95291
8 changed files with 336 additions and 27 deletions
@@ -569,11 +569,13 @@ async function promptAndStream(context: Readonly<{
);
}
const sourceHolder = createSourceHolder();
try {
const sourceResult = await resolveByteSource(
context.input,
context.input.signal,
context.options,
sourceHolder,
);
if (!sourceResult.ok) {
return observeResult(sourceResult, context.options.observer);
@@ -702,6 +704,10 @@ async function promptAndStream(context: Readonly<{
mapDownloadException(error),
context.options.observer,
);
} finally {
// TR-RR-04. Exactly once, on every path: success, validation failure,
// writer failure and abort.
closeHeldSource(sourceHolder);
}
}
@@ -724,12 +730,15 @@ async function boundedObjectUrlHandoff(context: Readonly<{
context.options.observer,
);
}
const sourceHolder = createSourceHolder();
const sourceResult = await resolveByteSource(
context.input,
context.input.signal,
context.options,
sourceHolder,
);
if (!sourceResult.ok) {
closeHeldSource(sourceHolder);
return observeResult(sourceResult, context.options.observer);
}
const source = sourceResult.value;
@@ -738,6 +747,7 @@ async function boundedObjectUrlHandoff(context: Readonly<{
(source.byteLength > context.input.maxBufferedBytes ||
source.byteLength > context.input.maxTransferBytes)
) {
closeHeldSource(sourceHolder);
return observeResult(
browserDataFailure("LIMIT_EXCEEDED", "DOWNLOAD"),
context.options.observer,
@@ -838,6 +848,9 @@ async function boundedObjectUrlHandoff(context: Readonly<{
context.options.observer,
transferred,
);
} finally {
// TR-RR-04. Exactly once, on every path.
closeHeldSource(sourceHolder);
}
}
@@ -997,10 +1010,37 @@ function validateDownloadInput(
return browserDataSuccess(true);
}
/**
* TR-RR-04. A presigned byte source owns a fetch reader and a capability lease,
* and its port requires `close()`. The delivery consumer never called it, so
* every success, validation failure, writer failure and abort leaked both. The
* closeable subtype is lost in the `FileByteSource` projection, so the holder
* keeps it and the outermost boundary closes it exactly once.
*/
type CloseableSourceHolder = { source: FileByteSource | null; closed: boolean };
function createSourceHolder(): CloseableSourceHolder {
return { source: null, closed: false };
}
function closeHeldSource(holder: CloseableSourceHolder): void {
if (holder.closed) return;
holder.closed = true;
const source = holder.source;
holder.source = null;
if (!source || !isVerifiedPresignedSource(source)) return;
try {
source.close();
} catch {
// Closing is best effort and never changes the delivery outcome.
}
}
async function resolveByteSource(
input: DeliveryInput,
signal: AbortSignal,
options: DownloadDeliveryAdapterOptions,
holder?: CloseableSourceHolder,
): Promise<BrowserDataResult<FileByteSource>> {
const source = input.source;
if (signal.aborted) {
@@ -1030,6 +1070,9 @@ async function resolveByteSource(
recovery: result.error.recovery,
});
}
// Held from the moment the lease exists, so a validation failure below still
// closes it.
if (holder) holder.source = result.value;
return validPresignedByteSource(
result.value,
source.capability,
@@ -323,6 +323,11 @@ export function createImageCdnRuntime(
);
}
activeCapabilityVerifications += 1;
// TR-RR-07. The slot belongs to the raw verifier, not to this wrapper.
// Releasing it when the wrapper's deadline expired let an abandoned
// verification keep running while a new one was admitted, so repeated
// timeouts produced more physical work than the configured cap allows.
const rawVerificationTasks: Promise<unknown>[] = [];
try {
const canonicalPayload =
canonicalImageCapabilityPayload(snapshot);
@@ -345,8 +350,10 @@ export function createImageCdnRuntime(
if (deadline.signal.aborted) {
throw capabilityVerificationAbortException();
}
const digestTask = sha256Hex(digest, canonicalPayload);
rawVerificationTasks.push(digestTask);
bindingDigest = await awaitImageRuntimeAbort(
sha256Hex(digest, canonicalPayload),
digestTask,
deadline.signal,
);
if (
@@ -363,14 +370,15 @@ export function createImageCdnRuntime(
if (deadline.signal.aborted) {
throw capabilityVerificationAbortException();
}
const verifyTask = verifyCapability({
algorithm: snapshot.signature.algorithm,
keyId: snapshot.signature.keyId,
canonicalPayload: Uint8Array.from(canonicalPayload),
signatureBase64Url: snapshot.signature.valueBase64Url,
});
rawVerificationTasks.push(verifyTask);
verified = await awaitImageRuntimeAbort(
verifyCapability({
algorithm: snapshot.signature.algorithm,
keyId: snapshot.signature.keyId,
canonicalPayload: Uint8Array.from(canonicalPayload),
signatureBase64Url:
snapshot.signature.valueBase64Url,
}),
verifyTask,
deadline.signal,
);
} catch {
@@ -427,7 +435,10 @@ export function createImageCdnRuntime(
);
return browserDataSuccess(reference);
} finally {
activeCapabilityVerifications -= 1;
// Released only once the physical work this slot admitted has settled.
void Promise.allSettled(rawVerificationTasks).then(() => {
activeCapabilityVerifications -= 1;
});
}
};
@@ -77,7 +77,12 @@ export type ResumableUploadRuntime = ResumableUploadPort &
* closes the checkpoint store and cancellation channel, so success actually
* means quiescent. No current bootstrap consumer is assumed.
*/
dispose(): Promise<void>;
/**
* TR-RR-06. Closes admission and returns the *bounded* drain result. A
* failure means the runtime is still `CLOSING`: physical work the caller
* must not treat as finished is still in flight.
*/
dispose(): Promise<BrowserDataResult<void>>;
lifecycle(): "OPEN" | "CLOSING" | "CLOSED";
}>;
@@ -178,7 +183,7 @@ export function createResumableUploadRuntime<Capability>(
const activeOperations = new Set<Promise<unknown>>();
let closed = false;
let lifecycle: "OPEN" | "CLOSING" | "CLOSED" = "OPEN";
let drain: Promise<void> | null = null;
let drain: Promise<BrowserDataResult<void>> | null = null;
const cancelLocalUploads = (uploadKey: string): void => {
if (!SAFE_UPLOAD_KEY.test(uploadKey)) return;
for (const controller of localUploads.get(uploadKey) ?? []) {
@@ -280,17 +285,19 @@ export function createResumableUploadRuntime<Capability>(
dependencies.crossContextCancellation?.publish(uploadKey);
}
const combined = combineAbortSignals(input.signal, lifetime.signal);
const operation = dependencies.mutationLock.run(
uploadKey,
combined.signal,
async () =>
await executeAbort(dependencies, uploadKey, combined.signal),
);
// TR-RR-06. An abort is admitted physical work like an upload, so it is
// tracked from admission and `dispose()` cannot step over it.
const tracked = Promise.resolve(operation).catch(() => undefined);
activeOperations.add(tracked);
void tracked.finally(() => activeOperations.delete(tracked));
try {
return await dependencies.mutationLock.run(
uploadKey,
combined.signal,
async () =>
await executeAbort(
dependencies,
uploadKey,
combined.signal,
),
);
return await operation;
} catch (error) {
return mapLockFailure(error, "UPLOAD_ABORT");
} finally {
@@ -304,14 +311,14 @@ export function createResumableUploadRuntime<Capability>(
void startDrain();
},
dispose(): Promise<void> {
dispose(): Promise<BrowserDataResult<void>> {
return startDrain();
},
lifecycle: () => lifecycle,
});
function startDrain(): Promise<void> {
function startDrain(): Promise<BrowserDataResult<void>> {
drain ??= (async () => {
closed = true;
lifecycle = "CLOSING";
@@ -322,10 +329,32 @@ export function createResumableUploadRuntime<Capability>(
for (const controllers of localUploads.values()) {
for (const controller of controllers) controller.abort();
}
await Promise.allSettled([...activeOperations]);
// TR-RR-06. Bounded. A non-cooperative mutation lock or provider must not
// make teardown unbounded, and an unproved drain is reported as such
// rather than closed over.
let timer: ReturnType<typeof setTimeout> | undefined;
const expired = new Promise<"EXPIRED">((resolve) => {
timer = setTimeout(
() => resolve("EXPIRED"),
dependencies.policy.cleanupDeadlineMs,
);
});
const drained = await Promise.race([
Promise.allSettled([...activeOperations]).then(() => "DRAINED" as const),
expired,
]);
if (timer !== undefined) clearTimeout(timer);
if (drained === "EXPIRED") {
// The store stays open: something can still write a checkpoint.
return browserDataFailure("UNAVAILABLE", "UPLOAD_ABORT", {
retryable: true,
recovery: "RESUME",
});
}
// The store closes only after nothing can still write a checkpoint.
dependencies.checkpoints.close();
lifecycle = "CLOSED";
return browserDataSuccess(undefined);
})();
return drain;
}
@@ -13,6 +13,12 @@ export type ResumableUploadRuntimePolicy = Readonly<{
capabilityRefreshSkewMs: number;
maxSessionLifetimeMs: number;
providerAttemptTimeoutMs: number;
/**
* TR-RR-06. The bound `dispose()` applies to its drain. A non-cooperative
* mutation lock or provider would otherwise make teardown unbounded, so a
* caller could never learn whether the runtime was quiescent.
*/
cleanupDeadlineMs: number;
}>;
const MIB = 1024 * 1024;
@@ -49,6 +55,7 @@ const DEFAULT_POLICY: ResumableUploadRuntimePolicy = Object.freeze({
capabilityRefreshSkewMs: 5_000,
maxSessionLifetimeMs: 24 * 60 * 60_000,
providerAttemptTimeoutMs: 30_000,
cleanupDeadlineMs: 10_000,
});
export function resolveResumableUploadRuntimePolicy(
@@ -95,6 +102,9 @@ export function resolveResumableUploadRuntimePolicy(
!positiveSafeInteger(policy.providerAttemptTimeoutMs) ||
policy.providerAttemptTimeoutMs >
ABSOLUTE_LIMITS.maxProviderAttemptTimeoutMs ||
!positiveSafeInteger(policy.cleanupDeadlineMs) ||
policy.cleanupDeadlineMs >
ABSOLUTE_LIMITS.maxProviderAttemptTimeoutMs * 2 ||
Math.ceil(policy.maxFileBytes / policy.partSizeBytes) >
policy.maxPartCount
) {