Skip to content

Commit 6721b2e

Browse files
committed
fix(logging): keep queued lines when the sink is reinstalled
A busy lock puts the lines back on the state install is about to replace, and that retry then sees a different state and returns. Carry the lines onto the new state and schedule the retry there.
1 parent 8a24757 commit 6721b2e

2 files changed

Lines changed: 39 additions & 2 deletions

File tree

‎src/logging/sink.test.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,6 +408,33 @@ describe("installFileLogSink", () => {
408408
}
409409
});
410410

411+
test("a reinstall while the log lock is held keeps queued lines", async () => {
412+
const { dir, file } = tempLogFile();
413+
const holder = liveLockHolder();
414+
try {
415+
installFileLogSink(file, { maxBytes: 10_000 });
416+
fs.writeFileSync(`${file}.lock`, `${holder.pid}:held`);
417+
getLogger(["logging", "reinstall"]).info("keep-me");
418+
installFileLogSink(file, { maxBytes: 10_000 });
419+
const before = fs.existsSync(file) ? fs.readFileSync(file, "utf8") : "";
420+
const rotatedBefore = fs.existsSync(`${file}.1`)
421+
? fs.readFileSync(`${file}.1`, "utf8")
422+
: "";
423+
expect(`${before}${rotatedBefore}`).not.toContain("keep-me");
424+
fs.rmSync(`${file}.lock`);
425+
await Promise.resolve();
426+
flushFileLogSinkSync();
427+
const active = fs.existsSync(file) ? fs.readFileSync(file, "utf8") : "";
428+
const rotated = fs.existsSync(`${file}.1`)
429+
? fs.readFileSync(`${file}.1`, "utf8")
430+
: "";
431+
expect(`${active}${rotated}`).toContain("keep-me");
432+
} finally {
433+
holder.kill();
434+
fs.rmSync(dir, { recursive: true, force: true });
435+
}
436+
});
437+
411438
test("a failed rotation keeps already-written records in the active file", () => {
412439
const { dir, file } = tempLogFile();
413440
try {

‎src/logging/sink.ts‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -500,20 +500,30 @@ export function installFileLogSink(
500500
options?: FileLogSinkOptions,
501501
): void {
502502
// A later install must not drop lines still queued for the previous path.
503+
// A busy lock puts them back and queues a retry on that state; replacing
504+
// state makes the retry return. Carry the lines onto the state that will
505+
// own them and schedule that retry.
503506
flushFileLogSinkSync();
507+
const carried = state === null ? [] : state.buffer;
508+
if (state !== null) {
509+
state.buffer = [];
510+
state.drainScheduled = false;
511+
}
504512
fs.mkdirSync(dirname(path), { recursive: true });
505513
const requested = options?.maxBytes;
506514
const initial = statActive(path);
507-
state = {
515+
const next: FileSinkState = {
508516
path,
509517
maxBytes:
510518
requested !== undefined && requested > 0 ? requested : LOG_MAX_BYTES,
511-
buffer: [],
519+
buffer: [...carried],
512520
size: initial.size,
513521
dev: initial.dev,
514522
ino: initial.ino,
515523
drainScheduled: false,
516524
};
525+
state = next;
526+
scheduleDrain(next);
517527
if (!exitHookInstalled) {
518528
exitHookInstalled = true;
519529
process.on("exit", flushFileLogSinkSync);

0 commit comments

Comments
 (0)