Skip to content

Commit 879d007

Browse files
committed
feat: US-113 - Add PTY signal callback error handling
1 parent 7bf422c commit 879d007

2 files changed

Lines changed: 56 additions & 1 deletion

File tree

packages/kernel/src/pty.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,13 @@ export class PtyManager {
390390
const signal = this.signalForByte(state, byte);
391391
if (signal !== null) {
392392
if (termios.icanon) state.lineBuffer.length = 0;
393-
if (state.foregroundPgid > 0) this.onSignal?.(state.foregroundPgid, signal);
393+
if (state.foregroundPgid > 0) {
394+
try {
395+
this.onSignal?.(state.foregroundPgid, signal);
396+
} catch {
397+
// Signal delivery failure must not break line discipline
398+
}
399+
}
394400
continue;
395401
}
396402
}

packages/kernel/test/resource-exhaustion.test.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -450,6 +450,55 @@ describe("PTY adversarial stress", () => {
450450
});
451451
});
452452

453+
describe("PTY signal callback error handling", () => {
454+
it("onSignal throw does not crash PTY — subsequent operations still work", async () => {
455+
const throwingSignalHandler = () => {
456+
throw new Error("signal handler exploded");
457+
};
458+
const manager = new PtyManager(throwingSignalHandler);
459+
const { master, slave } = manager.createPty();
460+
461+
// Configure canonical + echo + isig (defaults), set foreground pgid
462+
manager.setForegroundPgid(master.description.id, 42);
463+
464+
// Send ^C (0x03) — triggers onSignal which throws, should be caught internally
465+
expect(() =>
466+
manager.write(master.description.id, new Uint8Array([0x03])),
467+
).not.toThrow();
468+
469+
// PTY still functional — write and read through it
470+
manager.write(master.description.id, new Uint8Array([0x41, 0x0a])); // 'A\n'
471+
const echo = await manager.read(master.description.id, 1024);
472+
expect(echo).toContain(0x41); // 'A' echoed back
473+
474+
const input = await manager.read(slave.description.id, 1024);
475+
expect(input).toContain(0x41); // 'A' delivered to slave
476+
});
477+
478+
it("after failed signal delivery, echo and line discipline continue", async () => {
479+
const throwingSignalHandler = () => {
480+
throw new Error("boom");
481+
};
482+
const manager = new PtyManager(throwingSignalHandler);
483+
const { master, slave } = manager.createPty();
484+
485+
manager.setForegroundPgid(master.description.id, 99);
486+
487+
// Send multiple ^C characters — each should be caught, not accumulate errors
488+
const sigBytes = new Uint8Array([0x03, 0x03, 0x03]);
489+
expect(() => manager.write(master.description.id, sigBytes)).not.toThrow();
490+
491+
// Echo still works — write 'B' + newline
492+
manager.write(master.description.id, new Uint8Array([0x42, 0x0a])); // 'B\n'
493+
const echo = await manager.read(master.description.id, 1024);
494+
expect(echo).toContain(0x42);
495+
496+
// Read from slave — data delivered correctly
497+
const data = await manager.read(slave.description.id, 1024);
498+
expect(data).toContain(0x42);
499+
});
500+
});
501+
453502
describe("ID counter isolation", () => {
454503
it("100 FD descriptions, 100 pipes, 100 PTYs — all IDs unique, no range overlap", () => {
455504
const fdManager = new FDTableManager();

0 commit comments

Comments
 (0)