From 242eb30ab3bfd0e18936e71162262cd37e896092 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Mon, 28 Sep 2026 21:36:31 +0200 Subject: [PATCH] Keep an ended link ended, and pin the drop a listener closes over --- docs/decisions.md | 6 +++--- src/link-life.ts | 4 +++- test/session-extras.test.ts | 36 ++++++++++++++++++++++++++++++++++++ todo.md | 7 +++++++ 4 files changed, 49 insertions(+), 4 deletions(-) diff --git a/docs/decisions.md b/docs/decisions.md index 4dc690b..0310ea1 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -676,7 +676,7 @@ rule and an index of the titles below. the peer, whose every request is bounded by `responseTimeout` unless the caller set that to 0 as well, and unsafe for the application, which nothing bounds — `close()` is what you reach for when the application is stuck, so it may not block on the application coming unstuck. That half falls - back to `responseTimeout`, the same answer the link gate's hold already takes — and to that + back to `responseTimeout`, the same answer `LinkLife`'s hold already takes — and to that option's default where it is 0 as well, since neither option is an answer about the application. - **What the application holds unanswered is capped on constants, and a message past the cap is @@ -738,7 +738,7 @@ rule and an index of the titles below. optional so every construction site answers. `UnansweredError` stays unexported: `unanswered` is the one spelling on the public surface. The hold is bounded by `responseTimeout` rather than an option of its own — that is already the answer to how long one request may wait — and its clock - starts when the send is issued rather than when it first finds the gate shut, so one budget covers + starts when the send is issued rather than when it first finds the link down, so one budget covers every hold a single call makes. That timer is the one here that is not `unref()`'d: a held request is awaited with the socket already destroyed, so an unref'd one lets a process whose only remaining work is that send exit without settling it. @@ -757,7 +757,7 @@ rule and an index of the titles below. window is this end's own concurrency draining as the peer answers rather than a link going nowhere, and that bound would fail a message with more segments than `maxOutstanding` partway through against a slow peer. The failure is a plain `Error` rather than `UnansweredError`, the same answer - an abort at the gate already gives. The drain half needs nothing: `close({ signal })` already hands the signal to + an abort while held for a link already gives. The drain half needs nothing: `close({ signal })` already hands the signal to `window.idle()`, and `unbind()` taking none is the shape README states. - **`LinkLife` decides whether a link can carry a request, and a bind is what makes it one.** diff --git a/src/link-life.ts b/src/link-life.ts index 6bcaf2e..c9fe22d 100644 --- a/src/link-life.ts +++ b/src/link-life.ts @@ -91,8 +91,10 @@ export class LinkLife { return () => this.wait(deadline, signal); } - /** A socket from the reconnect loop, not yet bound. */ + /** A socket from the reconnect loop, not yet bound. An ended session stays ended. */ attach(): void { + if (this.isOver()) return; + this.phase = 'binding'; } diff --git a/test/session-extras.test.ts b/test/session-extras.test.ts index 682ac31..a6b75a1 100644 --- a/test/session-extras.test.ts +++ b/test/session-extras.test.ts @@ -1486,6 +1486,9 @@ describe('LinkLife', () => { link.end(); assert.match((await held).err?.message ?? '', /Session is closed/); + link.attach(); + assert.equal(link.isAttached(), false, 'ended is final'); + assert.equal(link.end(), false); }); }); @@ -2458,6 +2461,39 @@ describe('a peer that sends the next segment only once the last one is answered' assert.deepEqual(await peerOf(smpp).close(), {}, 'a group nothing completed is not held'); }); + test('reports a drop once as disconnected when a listener closes the session over the segments it lost', async t => { + const smpp = await startServer(t); + const { session } = await connect(t, smpp, { reconnect: { maxDelay: 100, minDelay: 20 } }); + + assert.ok(session); + + const events: string[] = []; + const closed = once(resolve => { session.on('close', () => { resolve(true); }); }); + + session.on('close', () => { events.push('close'); }); + session.on('disconnected', () => { events.push('disconnected'); }); + session.on('sessionError', () => { void session.close(); }); + + const [first] = segmentsOf(0x2D); + + assert.ok(first); + + const delivered = await peerOf(smpp).send({ + cmdName: 'deliver_sm', + params: submitSmParams( + { from: '46701113311', message: text, to: '46709771337' }, + first, + { encoding: 'ASCII', multipart: true }, + ), + }); + + assert.equal(delivered.err, undefined); + await peerOf(smpp).close(); + await closed; + + assert.deepEqual(events, ['disconnected', 'close']); + }); + test('close() still waits for a concatenated message the application has not answered', async t => { const smpp = await startServer(t, { shutdownTimeout: 50 }); const incoming = once(resolve => { diff --git a/todo.md b/todo.md index def24e5..03a79a7 100644 --- a/todo.md +++ b/todo.md @@ -211,6 +211,13 @@ hardest, and the held-message flow across `incoming-requests.ts`, `held-messages ### Correctness +- [ ] **Refuse to open a link that dropped while its rebind was answered.** A peer sending + `bind_resp` and FIN together can tear the link down before `comeBackUp()` resumes; it then + calls `link.open()` on a `down` link, `resetTimers()` skips, and `attempt()` reports success, + so the session is `up` on a destroyed socket with nothing to reconnect it until `close()`. + `open()` accepting only `binding`, and `comeBackUp()` returning an err when the link is no + longer attached, lets the loop retry. Unreproduced; from the stability review of #48. + - [ ] **Register a multipart send's receipt merge before its segments go out.** `Session.sendSms()` calls `dlrMerger.expect()` only once `submitSms()` resolves, after the last segment's response, so a receipt for an early segment that arrives first is logged at `debug` as naming no merge,