Skip to content

Commit 37d0850

Browse files
authored
Reüse server sockets across a reload. (#179)
This PR addresses the problem whereby a server socket passed as FD would prevent a successful system reload. This was because a reload _used_ to close all server sockets before restarting. However, this was a problem in that an FD-specified server socket couldn't actually be reopened. Now, _all_ server sockets are "stashed" during a reload operation instead of just closing them. If the new config wants them, it gets them back. But after a timeout of a few seconds, whatever wasn't used gets closed.
2 parents 7ebd118 + c8a2d68 commit 37d0850

File tree

3 files changed

+125
-10
lines changed

3 files changed

+125
-10
lines changed

src/network-protocol/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
"@this/loggy": "*",
1919
"@this/typey": "*",
2020
"express": "^4.18.1",
21-
"http2-express-bridge": "^1.0.7"
21+
"http2-express-bridge": "^1.0.7",
22+
"lodash": "^4.17.21"
2223
}
2324
}

src/network-protocol/private/AsyncServer.js

Lines changed: 122 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,12 @@
22
// SPDX-License-Identifier: Apache-2.0
33

44
import { Server, createServer as netCreateServer } from 'node:net';
5+
import { setTimeout } from 'node:timers/promises';
6+
7+
import lodash from 'lodash';
58

69
import { EventPayload, EventSource, LinkedEvent, PromiseUtil } from '@this/async';
7-
import { FormatUtils } from '@this/loggy';
10+
import { FormatUtils, IntfLogger } from '@this/loggy';
811
import { MustBe } from '@this/typey';
912

1013

@@ -13,6 +16,9 @@ import { MustBe } from '@this/typey';
1316
* in a way that is `async`-friendly.
1417
*/
1518
export class AsyncServer {
19+
/** @type {?IntfLogger} Logger to use, or `null` to not do any logging. */
20+
#logger;
21+
1622
/** @type {object} Parsed server socket `interface` specification. */
1723
#interface;
1824

@@ -39,11 +45,13 @@ export class AsyncServer {
3945
*
4046
* @param {object} iface Parsed server socket `interface` specification.
4147
* @param {string} protocol The protocol name; just used for logging.
48+
* @param {?IntfLogger} logger Logger to use, if any.
4249
*/
43-
constructor(iface, protocol) {
50+
constructor(iface, protocol, logger) {
4451
// Note: `interface` is a reserved word.
4552
this.#interface = MustBe.plainObject(iface);
4653
this.#protocol = MustBe.string(protocol);
54+
this.#logger = logger;
4755
}
4856

4957
/**
@@ -110,8 +118,33 @@ export class AsyncServer {
110118
async start(isReload) {
111119
MustBe.boolean(isReload);
112120

113-
this.#serverSocket = netCreateServer(
114-
AsyncServer.#extractConstructorOptions(this.#interface));
121+
// In case of a reload, look for a stashed instance which is already set up
122+
// the same way.
123+
const found = isReload
124+
? AsyncServer.#unstashInstance(this.#interface)
125+
: null;
126+
127+
if (found) {
128+
// Inherit the "guts" of the now-unstashed instance.
129+
this.#serverSocket = found.#serverSocket;
130+
this.#serverSocket.removeAllListeners();
131+
132+
// Transfer any unhandled events to the new instance.
133+
found.#eventSource.emit(new EventPayload('done'));
134+
let eventHead = await found.#eventHead;
135+
while (eventHead) {
136+
if (eventHead.type === 'done') {
137+
break;
138+
}
139+
this.emit(eventHead.payload);
140+
eventHead = eventHead.nextNow;
141+
}
142+
} else {
143+
// Either this isn't a reload, or it's a reload with an endpoint that
144+
// isn't configured the same way as one of the pre-reload ones.
145+
this.#serverSocket = netCreateServer(
146+
AsyncServer.#extractConstructorOptions(this.#interface));
147+
}
115148

116149
this.#serverSocket.on('connection', (...args) => {
117150
this.#eventSource.emit(new EventPayload('connection', ...args));
@@ -133,7 +166,11 @@ export class AsyncServer {
133166
async stop(willReload) {
134167
MustBe.boolean(willReload);
135168

136-
await this.#close();
169+
if (willReload) {
170+
AsyncServer.#stashInstance(this);
171+
} else {
172+
await this.#close();
173+
}
137174
}
138175

139176
/**
@@ -177,15 +214,35 @@ export class AsyncServer {
177214
serverSocket.on('error', handleError);
178215
});
179216
}
217+
218+
// Close any sockets that happened to have been accepted in this class but
219+
// which weren't then passed on to a client.
220+
// Transfer any unhandled events to the new instance.
221+
this.#eventSource.emit(new EventPayload('done'));
222+
let eventHead = await this.#eventHead;
223+
while (eventHead) {
224+
if (eventHead.type === 'done') {
225+
break;
226+
} else if (eventHead.type === 'connection') {
227+
const socket = eventHead.args[0];
228+
socket.destroy();
229+
}
230+
eventHead = eventHead.nextNow;
231+
}
232+
180233
}
181234

182235
/**
183-
* Performs a `listen()` on the underlying {@link Server}. This method
184-
* async-returns once the server is actually listening.
236+
* Performs a `listen()` on the underlying {@link Server}, if not already
237+
* done. This method async-returns once the server is actually listening.
185238
*/
186239
async #listen() {
187240
const serverSocket = this.#serverSocket;
188241

242+
if (serverSocket.listening) {
243+
return;
244+
}
245+
189246
// This `await new Promise` arrangement is done to get the `listen()` call
190247
// to be a good async citizen. Notably, the optional callback passed to
191248
// `listen()` is only ever sent a single `listening` event upon success and
@@ -217,7 +274,6 @@ export class AsyncServer {
217274
});
218275
}
219276

220-
221277
//
222278
// Static members
223279
//
@@ -246,6 +302,18 @@ export class AsyncServer {
246302
port: null
247303
});
248304

305+
/**
306+
* @type {number} How long in msec to allow a stashed instance to remain
307+
* stashed.
308+
*/
309+
static #STASH_TIMEOUT_MSEC = 5 * 1000;
310+
311+
/**
312+
* @type {Set<AsyncServer>} Set of stashed instances, for use during a reload.
313+
* Such instances were left open and listening during a previous `stop()`.
314+
*/
315+
static #stashedInstances = new Set();
316+
249317
/**
250318
* Gets the options for a `Server` constructor(ish) call, given the full
251319
* server socket `interface` options.
@@ -295,4 +363,50 @@ export class AsyncServer {
295363

296364
return result;
297365
}
366+
367+
/**
368+
* Stashes an instance for possible reuse during a reload.
369+
*
370+
* @param {AsyncServer} instance The instance to stash.
371+
*/
372+
static #stashInstance(instance) {
373+
// Remove any pre-existing matching instance. This shouldn't happen in the
374+
// first place, but if it does this will minimize the downstream confusion.
375+
this.#unstashInstance(instance.#interface);
376+
377+
this.#stashedInstances.add(instance);
378+
instance.#logger?.stashed();
379+
380+
(async () => {
381+
await setTimeout(this.#STASH_TIMEOUT_MSEC);
382+
if (this.#stashedInstances.delete(instance)) {
383+
instance.#logger?.stashTimeout();
384+
await instance.#close();
385+
}
386+
})();
387+
}
388+
389+
/**
390+
* Finds a matching instance of this class, if any, which was stashed away
391+
* during a reload. If found, it is removed from the stash.
392+
*
393+
* @param {object} iface The interface specification for the instance.
394+
* @returns {?AsyncServer} The found instance, if any.
395+
*/
396+
static #unstashInstance(iface) {
397+
let found = null;
398+
for (const si of this.#stashedInstances) {
399+
if (lodash.isEqual(iface, si.#interface)) {
400+
found = si;
401+
break;
402+
}
403+
}
404+
405+
if (found) {
406+
this.#stashedInstances.delete(found);
407+
found.#logger?.unstashed();
408+
}
409+
410+
return found;
411+
}
298412
}

src/network-protocol/private/TcpWrangler.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ export class TcpWrangler extends ProtocolWrangler {
4848

4949
this.#logger = options.logger ?? null;
5050
this.#rateLimiter = options.rateLimiter ?? null;
51-
this.#asyncServer = new AsyncServer(options.interface, options.protocol);
51+
this.#asyncServer = new AsyncServer(options.interface, options.protocol, this.#logger);
5252
}
5353

5454
/** @override */

0 commit comments

Comments
 (0)