Skip to content

Commit 8021cec

Browse files
committed
Retain overlapping scoped resources
Track active scoped configurations so scope disposal can retain disposable sinks and filters still owned by another live scope. This prevents concurrent or overlapping scopes that share a resource from closing it while another scope can still route logs to it. Prefer Symbol.asyncDispose over Symbol.dispose when a scoped sink or filter implements both disposal protocols, matching async resource ownership with a single cleanup path. #188 (comment) #188 (comment) #188 (comment) Assisted-by: Codex:gpt-5.5
1 parent 2fc375f commit 8021cec

3 files changed

Lines changed: 139 additions & 6 deletions

File tree

packages/logtape/src/config.test.ts

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -803,6 +803,41 @@ test("withConfig() disposes scoped resources when the scope exits", async () =>
803803
}
804804
});
805805

806+
test("withConfig() prefers async disposal for dual disposable resources", async () => {
807+
const events: string[] = [];
808+
const sink: Sink & Disposable & AsyncDisposable = () => {};
809+
sink[Symbol.dispose] = () => events.push("sync sink");
810+
sink[Symbol.asyncDispose] = async () => {
811+
await Promise.resolve();
812+
events.push("async sink");
813+
};
814+
const filter: Filter & Disposable & AsyncDisposable = () => true;
815+
filter[Symbol.dispose] = () => events.push("sync filter");
816+
filter[Symbol.asyncDispose] = async () => {
817+
await Promise.resolve();
818+
events.push("async filter");
819+
};
820+
821+
await configure({
822+
sinks: {},
823+
loggers: [{ category: ["logtape", "meta"], sinks: [] }],
824+
contextLocalStorage: new AsyncLocalStorage(),
825+
reset: true,
826+
});
827+
828+
try {
829+
await withConfig({
830+
sinks: { sink },
831+
filters: { filter },
832+
loggers: [{ category: "app", sinks: ["sink"], filters: ["filter"] }],
833+
}, () => {});
834+
835+
assert.deepStrictEqual(events, ["async filter", "async sink"]);
836+
} finally {
837+
await reset();
838+
}
839+
});
840+
806841
test("withConfig() does not dispose resources still owned by parent scopes", async () => {
807842
const records: LogRecord[] = [];
808843
const events: string[] = [];
@@ -852,6 +887,70 @@ test("withConfig() does not dispose resources still owned by parent scopes", asy
852887
}
853888
});
854889

890+
test("withConfig() does not dispose resources still owned by sibling scopes", async () => {
891+
const records: LogRecord[] = [];
892+
const events: string[] = [];
893+
const sharedSink: Sink & Disposable = (record) => {
894+
records.push(record);
895+
};
896+
sharedSink[Symbol.dispose] = () => events.push("sink");
897+
const sharedFilter: Filter & Disposable = () => true;
898+
sharedFilter[Symbol.dispose] = () => events.push("filter");
899+
900+
await configure({
901+
sinks: {},
902+
loggers: [{ category: ["logtape", "meta"], sinks: [] }],
903+
contextLocalStorage: new AsyncLocalStorage(),
904+
reset: true,
905+
});
906+
907+
try {
908+
let finishFirst!: () => void;
909+
let markSecondEntered!: () => void;
910+
const releaseFirst = new Promise<void>((resolve) => {
911+
finishFirst = resolve;
912+
});
913+
const secondEntered = new Promise<void>((resolve) => {
914+
markSecondEntered = resolve;
915+
});
916+
const firstDone = withConfig({
917+
sinks: { shared: sharedSink },
918+
filters: { shared: sharedFilter },
919+
loggers: [
920+
{ category: "app", filters: ["shared"], sinks: ["shared"] },
921+
],
922+
}, async () => {
923+
await secondEntered;
924+
await releaseFirst;
925+
getLogger("app").info("first");
926+
});
927+
const secondDone = withConfig({
928+
sinks: { shared: sharedSink },
929+
filters: { shared: sharedFilter },
930+
loggers: [
931+
{ category: "app", filters: ["shared"], sinks: ["shared"] },
932+
],
933+
}, () => {
934+
markSecondEntered();
935+
assert.deepStrictEqual(events, []);
936+
getLogger("app").info("second");
937+
});
938+
939+
await secondDone;
940+
assert.deepStrictEqual(events, []);
941+
finishFirst();
942+
await firstDone;
943+
944+
assert.deepStrictEqual(events, ["filter", "sink"]);
945+
assert.deepStrictEqual(
946+
records.map((record) => record.rawMessage).sort(),
947+
["first", "second"],
948+
);
949+
} finally {
950+
await reset();
951+
}
952+
});
953+
855954
test("withConfig() does not dispose resources still owned globally", async () => {
856955
const records: LogRecord[] = [];
857956
const events: string[] = [];

packages/logtape/src/config.ts

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { type FilterLike, toFilter } from "./filter.ts";
33
import type { LogLevel } from "./level.ts";
44
import { LoggerImpl } from "./logger.ts";
55
import {
6+
type CompiledScopedConfig,
67
compileScopedConfig,
78
disposeScopedConfig,
89
disposeScopedConfigSync,
@@ -107,6 +108,7 @@ export interface LoggerConfig<
107108
*/
108109
let currentConfig: Config<string, string> | null = null;
109110
let activeScopedConfigCount = 0;
111+
const activeScopedConfigs: Set<CompiledScopedConfig> = new Set();
110112
let globalConfigMutationInProgress = false;
111113

112114
/**
@@ -339,6 +341,7 @@ export async function withConfig<
339341
let callbackError: unknown;
340342
let callbackFailed = false;
341343
activeScopedConfigCount++;
344+
activeScopedConfigs.add(scopedConfig);
342345
try {
343346
result = await runWithScopedConfig(
344347
contextLocalStorage,
@@ -351,13 +354,17 @@ export async function withConfig<
351354
}
352355

353356
try {
354-
await disposeScopedConfig(scopedConfig, getGlobalDisposables());
357+
await disposeScopedConfig(
358+
scopedConfig,
359+
getRetainedDisposables(scopedConfig),
360+
);
355361
} catch (disposeError) {
356362
if (callbackFailed) {
357363
throwCombinedErrors(callbackError, disposeError);
358364
}
359365
throw disposeError;
360366
} finally {
367+
activeScopedConfigs.delete(scopedConfig);
361368
activeScopedConfigCount--;
362369
}
363370

@@ -397,6 +404,7 @@ export function withConfigSync<
397404
let callbackError: unknown;
398405
let callbackFailed = false;
399406
activeScopedConfigCount++;
407+
activeScopedConfigs.add(scopedConfig);
400408
try {
401409
result = runWithScopedConfig(contextLocalStorage, scopedConfig, callback);
402410
if (isThenable(result)) {
@@ -413,13 +421,14 @@ export function withConfigSync<
413421
}
414422

415423
try {
416-
disposeScopedConfigSync(scopedConfig, getGlobalDisposables());
424+
disposeScopedConfigSync(scopedConfig, getRetainedDisposables(scopedConfig));
417425
} catch (disposeError) {
418426
if (callbackFailed) {
419427
throwCombinedErrors(callbackError, disposeError);
420428
}
421429
throw disposeError;
422430
} finally {
431+
activeScopedConfigs.delete(scopedConfig);
423432
activeScopedConfigCount--;
424433
}
425434

@@ -443,6 +452,33 @@ function getGlobalDisposables(): ReadonlySet<Disposable | AsyncDisposable> {
443452
]);
444453
}
445454

455+
function getRetainedDisposables(
456+
scopedConfig: CompiledScopedConfig,
457+
): ReadonlySet<Disposable | AsyncDisposable> {
458+
const disposables = new Set<Disposable | AsyncDisposable>(
459+
getGlobalDisposables(),
460+
);
461+
for (const activeScopedConfig of activeScopedConfigs) {
462+
if (activeScopedConfig === scopedConfig) continue;
463+
addScopedConfigDisposables(disposables, activeScopedConfig);
464+
}
465+
return disposables;
466+
}
467+
468+
function addScopedConfigDisposables(
469+
disposables: Set<Disposable | AsyncDisposable>,
470+
scopedConfig: CompiledScopedConfig,
471+
): void {
472+
for (const disposable of scopedConfig.syncFilters) {
473+
disposables.add(disposable);
474+
}
475+
for (const disposable of scopedConfig.asyncFilters) {
476+
disposables.add(disposable);
477+
}
478+
for (const disposable of scopedConfig.syncSinks) disposables.add(disposable);
479+
for (const disposable of scopedConfig.asyncSinks) disposables.add(disposable);
480+
}
481+
446482
function configureInternal<
447483
TSinkId extends string,
448484
TFilterId extends string,

packages/logtape/src/scoped-config.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,7 @@ export function compileScopedConfig<
141141
);
142142
}
143143
asyncSinks.add(sink as AsyncDisposable);
144-
}
145-
if (Symbol.dispose in sink) syncSinks.add(sink as Disposable);
144+
} else if (Symbol.dispose in sink) syncSinks.add(sink as Disposable);
146145
}
147146

148147
for (const filter of Object.values<FilterLike>(config.filters ?? {})) {
@@ -155,8 +154,7 @@ export function compileScopedConfig<
155154
}
156155
asyncFilters.add(filter as AsyncDisposable);
157156
asyncSinks.delete(filter as AsyncDisposable);
158-
}
159-
if (Symbol.dispose in filter) {
157+
} else if (Symbol.dispose in filter) {
160158
syncFilters.add(filter as Disposable);
161159
syncSinks.delete(filter as Disposable);
162160
}

0 commit comments

Comments
 (0)