Skip to content

Commit cc169d1

Browse files
committed
🐛 Fix race condition in lambda.register with once option
Ensure the lambda is unregistered before invoking the function when `once` option is specified, preventing multiple concurrent calls from executing the function more than once. Update test to cover the case where the lambda is called multiple times before the first call resolves.
1 parent 5f8e7f4 commit cc169d1

File tree

3 files changed

+16
-10
lines changed

3 files changed

+16
-10
lines changed

deno.jsonc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
]
4848
},
4949
"imports": {
50+
"@core/asyncutil": "jsr:@core/asyncutil@^1.2.0",
5051
"@core/unknownutil": "jsr:@core/unknownutil@^4.3.0",
5152
"@denops/core": "jsr:@denops/core@^7.0.0",
5253
"@denops/test": "jsr:@denops/test@^3.0.1",

lambda/mod.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -120,11 +120,8 @@ export function register(
120120
const id = `lambda:${denops.name}:${uuid}`;
121121
if (options.once) {
122122
denops.dispatcher[id] = async (...args: unknown[]) => {
123-
try {
124-
return await fn(...args);
125-
} finally {
126-
unregister(denops, id);
127-
}
123+
unregister(denops, id);
124+
return await fn(...args);
128125
};
129126
} else {
130127
denops.dispatcher[id] = async (...args: unknown[]) => {

lambda/mod_test.ts

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
returnsNext,
77
spy,
88
} from "@std/testing/mock";
9+
import { flushPromises } from "@core/asyncutil";
910
import { assert, is } from "@core/unknownutil";
1011
import { test } from "@denops/test";
1112
import { expr, rawString } from "../eval/mod.ts";
@@ -72,16 +73,23 @@ test({
7273
});
7374
await t.step("if 'once' option is specified", async (t) => {
7475
await t.step("registers an oneshot lambda function", async (t) => {
75-
const fn = spy(returnsNext(["foo"]));
76+
const waiter = Promise.withResolvers<void>();
77+
const fn = spy(resolvesNext([waiter.promise.then(() => "foo")]));
7678
const id = lambda.register(denops, fn, { once: true });
7779
assertSpyCalls(fn, 0);
78-
assertEquals(await denops.dispatch(denops.name, id), "foo");
80+
81+
// Call the lambda function twice before the first call resolves
82+
const firstCall = denops.dispatch(denops.name, id);
83+
const secondCall = denops.dispatch(denops.name, id);
84+
secondCall.catch(NOOP);
85+
await flushPromises();
86+
waiter.resolve();
87+
88+
assertEquals(await firstCall, "foo");
7989
assertSpyCalls(fn, 1);
8090

8191
await t.step("which will be removed if called once", async () => {
82-
const error = await assertRejects(
83-
() => denops.dispatch(denops.name, id),
84-
);
92+
const error = await assertRejects(() => secondCall);
8593
assertStringIncludes(
8694
error as string,
8795
"denops.dispatcher[name] is not a function",

0 commit comments

Comments
 (0)