Skip to content

Commit 940a480

Browse files
Millylambdalisue
authored andcommitted
👍 gracefully close service
- The Service is closed when The worker is terminated. - The worker is teminated when The Service is closed. - `denops.dispatch()` (but `service.dispatch()`) rejects with a service closed error if the service is closed before the plugin loaded.
1 parent 4d319d7 commit 940a480

File tree

4 files changed

+231
-11
lines changed

4 files changed

+231
-11
lines changed

denops/@denops-private/service.ts

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,14 @@ import type { CallbackId, Service as HostService } from "./host.ts";
77
/**
88
* Service manage plugins and is visible from the host (Vim/Neovim) through `invoke()` function.
99
*/
10-
export class Service implements HostService, Disposable {
10+
export class Service implements HostService, AsyncDisposable {
1111
#interruptController = new AbortController();
1212
#plugins = new Map<string, Plugin>();
1313
#waiters = new Map<string, PromiseWithResolvers<void>>();
1414
#meta: Meta;
1515
#host?: Host;
16+
#closed = false;
17+
#closedWaiter = Promise.withResolvers<void>();
1618

1719
constructor(meta: Meta) {
1820
this.#meta = meta;
@@ -22,6 +24,7 @@ export class Service implements HostService, Disposable {
2224
let waiter = this.#waiters.get(name);
2325
if (!waiter) {
2426
waiter = Promise.withResolvers();
27+
waiter.promise.catch(() => {});
2528
this.#waiters.set(name, waiter);
2629
}
2730
return waiter;
@@ -40,6 +43,9 @@ export class Service implements HostService, Disposable {
4043
script: string,
4144
suffix = "",
4245
): Promise<void> {
46+
if (this.#closed) {
47+
throw new Error("Service closed");
48+
}
4349
if (!this.#host) {
4450
throw new Error("No host is bound to the service");
4551
}
@@ -75,6 +81,9 @@ export class Service implements HostService, Disposable {
7581
}
7682

7783
waitLoaded(name: string): Promise<void> {
84+
if (this.#closed) {
85+
return Promise.reject(new Error("Service closed"));
86+
}
7887
return this.#getWaiter(name).promise;
7988
}
8089

@@ -128,8 +137,27 @@ export class Service implements HostService, Disposable {
128137
}
129138
}
130139

131-
[Symbol.dispose](): void {
132-
this.#plugins.clear();
140+
close(): Promise<void> {
141+
if (!this.#closed) {
142+
this.#closed = true;
143+
const error = new Error("Service closed");
144+
for (const { reject } of this.#waiters.values()) {
145+
reject(error);
146+
}
147+
this.#waiters.clear();
148+
this.#plugins.clear();
149+
this.#host = void 0;
150+
this.#closedWaiter.resolve();
151+
}
152+
return this.waitClosed();
153+
}
154+
155+
waitClosed(): Promise<void> {
156+
return this.#closedWaiter.promise;
157+
}
158+
159+
[Symbol.asyncDispose](): Promise<void> {
160+
return this.close();
133161
}
134162
}
135163

denops/@denops-private/service_test.ts

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -506,4 +506,79 @@ Deno.test("Service", async (t) => {
506506
assertThrows(() => signal.throwIfAborted(), "test");
507507
},
508508
);
509+
510+
const waitClosed = service.waitClosed();
511+
512+
const waitLoadedCalledBeforeClose = service.waitLoaded(
513+
"whenclosedtestplugin",
514+
);
515+
516+
await t.step(
517+
"the result promise of waitClosed() become 'pending' when the service is not closed",
518+
async () => {
519+
assert(await promiseState(waitClosed), "pending");
520+
},
521+
);
522+
523+
await t.step(
524+
"close() closes the service",
525+
async () => {
526+
await service.close();
527+
},
528+
);
529+
530+
await t.step(
531+
"the result promise of waitClosed() become 'fulfilled' when the service is closed",
532+
async () => {
533+
assert(await promiseState(waitClosed), "fulfilled");
534+
},
535+
);
536+
537+
await t.step(
538+
"the result promise of waitLoaded() become 'rejected' when the service is closed",
539+
async () => {
540+
assertEquals(await promiseState(waitLoadedCalledBeforeClose), "rejected");
541+
await assertRejects(
542+
() => waitLoadedCalledBeforeClose,
543+
Error,
544+
"Service closed",
545+
);
546+
},
547+
);
548+
549+
await t.step(
550+
"waitClosed() returns 'fulfilled' promise if the service is already closed",
551+
async () => {
552+
const actual = service.waitClosed();
553+
assert(await promiseState(actual), "fulfilled");
554+
},
555+
);
556+
557+
await t.step(
558+
"waitLoaded() returns 'rejected' promise if the service is already closed",
559+
async () => {
560+
await assertRejects(
561+
() => service.waitLoaded("after-closed-test-plugin"),
562+
Error,
563+
"Service closed",
564+
);
565+
},
566+
);
567+
568+
await t.step(
569+
"load() rejects an error when the service is already closed",
570+
async () => {
571+
await assertRejects(
572+
() => service.load("dummyValid", scriptValid),
573+
Error,
574+
"Service closed",
575+
);
576+
},
577+
);
578+
579+
await t.step("[@@asyncDispose]() calls close()", async () => {
580+
using service_close = stub(service, "close");
581+
await service[Symbol.asyncDispose]();
582+
assertSpyCalls(service_close, 1);
583+
});
509584
});

denops/@denops-private/worker.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,11 @@ async function connectHost(): Promise<void> {
7777

7878
// Start service
7979
using sigintTrap = asyncSignal("SIGINT");
80-
using service = new Service(meta);
80+
await using service = new Service(meta);
8181
await host.init(service);
8282
await host.call("execute", "doautocmd <nomodeline> User DenopsReady", "");
8383
await Promise.race([
84+
service.waitClosed(),
8485
host.waitClosed(),
8586
sigintTrap,
8687
]);

denops/@denops-private/worker_test.ts

Lines changed: 123 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
import {
1010
assertSpyCalls,
1111
resolvesNext,
12+
spy,
1213
stub,
1314
} from "jsr:@std/[email protected]/mock";
1415
import { delay } from "jsr:@std/[email protected]/delay";
@@ -17,6 +18,7 @@ import * as nvimCodec from "jsr:@lambdalisue/messagepack@^1.0.1";
1718
import { createFakeMeta } from "./testutil/mock.ts";
1819
import { Neovim } from "./host/nvim.ts";
1920
import { Vim } from "./host/vim.ts";
21+
import { Service } from "./service.ts";
2022
import { main } from "./worker.ts";
2123

2224
const CONSOLE_PATCH_METHODS = [
@@ -125,6 +127,14 @@ for (const { host, mode } of matrix) {
125127
using _addEventListenerSpy = spyAddEventListener();
126128
using _denoCommandStub = stubDenoCommand();
127129
using deno_addSignalListener = stub(Deno, "addSignalListener");
130+
using host_asyncDispose = spy(
131+
(host === "vim" ? Vim : Neovim).prototype,
132+
Symbol.asyncDispose,
133+
);
134+
using service_asyncDispose = spy(
135+
Service.prototype,
136+
Symbol.asyncDispose,
137+
);
128138
using self_close = stub(globalThis, "close");
129139
const usePostMessageHistory = () => ({
130140
[Symbol.dispose]: () => messageStub.postMessage.resetHistory(),
@@ -353,15 +363,37 @@ for (const { host, mode } of matrix) {
353363
assertInstanceOf(signalHandler, Function);
354364
});
355365

356-
await t.step("closes worker when stream is closed", async () => {
357-
assertSpyCalls(self_close, 0);
366+
await t.step("before stream is closed", async (t) => {
367+
await t.step("does not dispose service", () => {
368+
assertSpyCalls(service_asyncDispose, 0);
369+
});
358370

359-
// NOTE: Send `null` to close workerio stream.
360-
messageStub.fakeHostMessage(null);
371+
await t.step("does not dispose host", () => {
372+
assertSpyCalls(host_asyncDispose, 0);
373+
});
361374

362-
await delay(0);
363-
assertSpyCalls(self_close, 1);
364-
await mainPromise;
375+
await t.step("does not close worker", () => {
376+
assertSpyCalls(self_close, 0);
377+
});
378+
});
379+
380+
// NOTE: Send `null` to close workerio stream.
381+
messageStub.fakeHostMessage(null);
382+
await delay(0);
383+
384+
await t.step("after stream is closed", async (t) => {
385+
await t.step("disposes service", () => {
386+
assertSpyCalls(service_asyncDispose, 1);
387+
});
388+
389+
await t.step("disposes host", () => {
390+
assertSpyCalls(host_asyncDispose, 1);
391+
});
392+
393+
await t.step("closes worker", async () => {
394+
assertSpyCalls(self_close, 1);
395+
await mainPromise;
396+
});
365397
});
366398
});
367399

@@ -397,6 +429,14 @@ for (const { host, mode } of matrix) {
397429
using _addEventListenerSpy = spyAddEventListener();
398430
using _denoCommandStub = stubDenoCommand();
399431
using deno_addSignalListener = stub(Deno, "addSignalListener");
432+
using host_asyncDispose = spy(
433+
(host === "vim" ? Vim : Neovim).prototype,
434+
Symbol.asyncDispose,
435+
);
436+
using service_asyncDispose = spy(
437+
Service.prototype,
438+
Symbol.asyncDispose,
439+
);
400440
using self_close = stub(globalThis, "close");
401441
const fakeMeta = { ...createFakeMeta(), host, mode };
402442

@@ -441,6 +481,82 @@ for (const { host, mode } of matrix) {
441481
);
442482
});
443483

484+
await t.step("disposes service", () => {
485+
assertSpyCalls(service_asyncDispose, 1);
486+
});
487+
488+
await t.step("disposes host", () => {
489+
assertSpyCalls(host_asyncDispose, 1);
490+
});
491+
492+
await t.step("closes worker", async () => {
493+
assertSpyCalls(self_close, 1);
494+
await mainPromise;
495+
});
496+
});
497+
498+
await t.step("main() if service is closed", async (t) => {
499+
using messageStub = stubMessage();
500+
using _consoleStub = stubConsole();
501+
using _addEventListenerSpy = spyAddEventListener();
502+
using _denoCommandStub = stubDenoCommand();
503+
using _deno_addSignalListener = stub(Deno, "addSignalListener");
504+
using host_asyncDispose = spy(
505+
(host === "vim" ? Vim : Neovim).prototype,
506+
Symbol.asyncDispose,
507+
);
508+
using service_asyncDispose = spy(
509+
Service.prototype,
510+
Symbol.asyncDispose,
511+
);
512+
const service_waitClosed_waiter = Promise.withResolvers<void>();
513+
using service_waitClosed = stub(
514+
Service.prototype,
515+
"waitClosed",
516+
() => service_waitClosed_waiter.promise,
517+
);
518+
using self_close = stub(globalThis, "close");
519+
const fakeMeta = { ...createFakeMeta(), host, mode };
520+
521+
const mainPromise = main();
522+
523+
if (host === "vim") {
524+
// Initial message from the host.
525+
messageStub.fakeHostMessage(vimCodec.encode([0, ["void"]]));
526+
await delay(0);
527+
// requests Meta data
528+
messageStub.fakeHostMessage(vimCodec.encode([-1, [fakeMeta, ""]]));
529+
await delay(0);
530+
// doautocmd `User DenopsReady`
531+
messageStub.fakeHostMessage(vimCodec.encode([-2, ["", ""]]));
532+
await delay(0);
533+
} else {
534+
// Initial message from the host.
535+
messageStub.fakeHostMessage(nvimCodec.encode([2, "void", []]));
536+
await delay(0);
537+
// requests Meta data
538+
messageStub.fakeHostMessage(nvimCodec.encode([1, 0, null, fakeMeta]));
539+
await delay(0);
540+
// sets client info
541+
messageStub.fakeHostMessage(nvimCodec.encode([1, 1, null, 0]));
542+
await delay(0);
543+
// doautocmd `User DenopsReady`
544+
messageStub.fakeHostMessage(nvimCodec.encode([1, 2, null, ""]));
545+
await delay(0);
546+
}
547+
548+
assertSpyCalls(service_waitClosed, 1);
549+
service_waitClosed_waiter.resolve();
550+
await delay(0);
551+
552+
await t.step("disposes service", () => {
553+
assertSpyCalls(service_asyncDispose, 1);
554+
});
555+
556+
await t.step("disposes host", () => {
557+
assertSpyCalls(host_asyncDispose, 1);
558+
});
559+
444560
await t.step("closes worker", async () => {
445561
assertSpyCalls(self_close, 1);
446562
await mainPromise;

0 commit comments

Comments
 (0)