Explorar o código

fix(grep): harden auto-install concurrency

Prevent one caller from cancelling a shared ripgrep install for other waiters, and drain tar stderr concurrently during extraction to avoid pipe backpressure deadlocks.
dhaern hai 3 meses
pai
achega
a742009fb9
Modificáronse 3 ficheiros con 222 adicións e 24 borrados
  1. 18 3
      src/tools/grep/downloader.ts
  2. 120 0
      src/tools/grep/resolver.test.ts
  3. 84 21
      src/tools/grep/resolver.ts

+ 18 - 3
src/tools/grep/downloader.ts

@@ -12,7 +12,7 @@ import { homedir } from 'node:os';
 import { dirname, join } from 'node:path';
 import { sync as whichSync } from 'which';
 import { extractZip, getZipExtractionSupportError } from '../../utils';
-import { crossSpawn } from '../../utils/compat';
+import { type CrossSpawnResult, crossSpawn } from '../../utils/compat';
 
 interface RipgrepReleaseAsset {
   name?: string;
@@ -43,6 +43,21 @@ function throwIfAborted(signal?: AbortSignal): void {
   }
 }
 
+async function waitForExitAndStderr(
+  proc: CrossSpawnResult,
+  stderrPromise: Promise<string>,
+): Promise<{ exitCode: number; stderr: string }> {
+  const [exitResult, stderrResult] = await Promise.allSettled([
+    proc.exited,
+    stderrPromise,
+  ]);
+
+  return {
+    exitCode: exitResult.status === 'fulfilled' ? exitResult.value : 1,
+    stderr: stderrResult.status === 'fulfilled' ? stderrResult.value : '',
+  };
+}
+
 function hasExecutable(name: string): boolean {
   try {
     const resolved = whichSync(name, { nothrow: true });
@@ -289,7 +304,8 @@ async function extractTarGz(
 
   signal?.addEventListener('abort', onAbort, { once: true });
 
-  const exitCode = await proc.exited;
+  const stderrPromise = proc.stderr();
+  const { exitCode, stderr } = await waitForExitAndStderr(proc, stderrPromise);
   signal?.removeEventListener('abort', onAbort);
 
   if (signal?.aborted) {
@@ -297,7 +313,6 @@ async function extractTarGz(
   }
 
   if (exitCode !== 0) {
-    const stderr = await proc.stderr();
     throw new Error(`ripgrep extraction failed (exit ${exitCode}): ${stderr}`);
   }
 }

+ 120 - 0
src/tools/grep/resolver.test.ts

@@ -189,6 +189,126 @@ describe('tools/grep/resolver', () => {
     expect(secondAttempt.source).toBe('managed-rg');
   });
 
+  test('resolveGrepCliWithAutoInstall does not let one caller abort a shared install for another waiter', async () => {
+    let installSignal: AbortSignal | undefined;
+    let resolveInstall: ((path: string) => void) | undefined;
+    let rejectInstall: ((error: Error) => void) | undefined;
+    let markStarted: (() => void) | undefined;
+    const started = new Promise<void>((resolve) => {
+      markStarted = resolve;
+    });
+
+    const firstController = new AbortController();
+    const installLatest = mock((signal?: AbortSignal) => {
+      installSignal = signal;
+      markStarted?.();
+      return new Promise<string>((resolve, reject) => {
+        resolveInstall = resolve;
+        rejectInstall = reject;
+        signal?.addEventListener(
+          'abort',
+          () => {
+            const error = new Error('aborted');
+            error.name = 'AbortError';
+            reject(error);
+          },
+          { once: true },
+        );
+      });
+    });
+
+    const deps = {
+      findExecutable: () => null,
+      getInstalledRipgrepPath: () => null,
+      installLatestStableRipgrep: installLatest,
+      logger: () => undefined,
+    };
+
+    const firstWaiter = resolveGrepCliWithAutoInstall(deps, firstController.signal);
+    await started;
+    const secondWaiter = resolveGrepCliWithAutoInstall(deps);
+
+    firstController.abort();
+
+    await expect(firstWaiter).rejects.toThrow(
+      /cancelled before execution started/i,
+    );
+    expect(installLatest.mock.calls).toHaveLength(1);
+    expect(installSignal?.aborted).toBe(false);
+
+    resolveInstall?.('/tmp/managed-rg');
+    await expect(secondWaiter).resolves.toEqual({
+      path: '/tmp/managed-rg',
+      backend: 'rg',
+      source: 'managed-rg',
+    });
+
+    expect(installSignal?.aborted).toBe(false);
+    expect(rejectInstall).toBeDefined();
+  });
+
+  test('resolveGrepCliWithAutoInstall aborts the shared install when the last waiter cancels', async () => {
+    let installSignal: AbortSignal | undefined;
+    let markStarted: (() => void) | undefined;
+    let markAborted: (() => void) | undefined;
+    const started = new Promise<void>((resolve) => {
+      markStarted = resolve;
+    });
+    const aborted = new Promise<void>((resolve) => {
+      markAborted = resolve;
+    });
+
+    const controller = new AbortController();
+
+    const firstAttempt = resolveGrepCliWithAutoInstall(
+      {
+        findExecutable: () => null,
+        getInstalledRipgrepPath: () => null,
+        installLatestStableRipgrep: async (signal?: AbortSignal) => {
+          installSignal = signal;
+          markStarted?.();
+          await new Promise<never>((_, reject) => {
+            signal?.addEventListener(
+              'abort',
+              () => {
+                markAborted?.();
+                const error = new Error('aborted');
+                error.name = 'AbortError';
+                reject(error);
+              },
+              { once: true },
+            );
+          });
+          return '/tmp/unreachable';
+        },
+        logger: () => undefined,
+      },
+      controller.signal,
+    );
+
+    await started;
+    controller.abort();
+
+    await expect(firstAttempt).rejects.toThrow(
+      /cancelled before execution started/i,
+    );
+    await aborted;
+    expect(installSignal?.aborted).toBe(true);
+
+    const retry = await resolveGrepCliWithAutoInstall({
+      findExecutable: () => null,
+      getInstalledRipgrepPath: () => null,
+      installLatestStableRipgrep: async () => '/tmp/managed-rg',
+      logger: () => undefined,
+    });
+
+    expect(retry).toEqual({
+      path: '/tmp/managed-rg',
+      backend: 'rg',
+      source: 'managed-rg',
+    });
+  });
+
   test('resolveGrepCliWithAutoInstall throws a clear error when rg and grep are unavailable', async () => {
     await expect(
       resolveGrepCliWithAutoInstall({

+ 84 - 21
src/tools/grep/resolver.ts

@@ -23,7 +23,14 @@ interface GrepResolverDependencies {
   logger?: (message: string, data?: unknown) => void;
 }
 
-let autoInstallPromise: Promise<ResolvedGrepCli> | null = null;
+interface SharedAutoInstallState {
+  promise: Promise<ResolvedGrepCli>;
+  controller: AbortController;
+  waiters: number;
+  settled: boolean;
+}
+
+let autoInstallState: SharedAutoInstallState | null = null;
 
 function defaultFindExecutable(name: string): string | null {
   try {
@@ -152,36 +159,66 @@ function raceWithAbort<T>(
   });
 }
 
-export async function resolveGrepCliWithAutoInstall(
-  deps: GrepResolverDependencies = {},
-  signal?: AbortSignal,
-): Promise<ResolvedGrepCli> {
-  if (signal?.aborted) {
-    throw new AbortWaitError('Search was cancelled before execution started.');
-  }
+function releaseAutoInstallWaiter(state: SharedAutoInstallState): void {
+  state.waiters = Math.max(0, state.waiters - 1);
 
-  const current = resolveSync(deps);
-  if (isResolvedRipgrep(current)) {
-    return current;
+  if (state.waiters > 0 || state.settled) {
+    return;
   }
 
-  if (autoInstallPromise) {
-    return raceWithAbort(autoInstallPromise, signal);
+  if (autoInstallState === state) {
+    autoInstallState = null;
   }
 
-  autoInstallPromise = (async () => {
-    const installManagedRipgrep =
-      deps.installLatestStableRipgrep ?? installLatestStableRipgrep;
+  state.controller.abort();
+}
+
+function waitForSharedAutoInstall(
+  state: SharedAutoInstallState,
+  signal?: AbortSignal,
+): Promise<ResolvedGrepCli> {
+  state.waiters += 1;
+
+  let released = false;
+  const release = () => {
+    if (released) {
+      return;
+    }
+
+    released = true;
+    releaseAutoInstallWaiter(state);
+  };
+
+  return raceWithAbort(state.promise, signal).finally(release);
+}
+
+function createSharedAutoInstall(
+  deps: GrepResolverDependencies,
+): SharedAutoInstallState {
+  const installManagedRipgrep =
+    deps.installLatestStableRipgrep ?? installLatestStableRipgrep;
+  const controller = new AbortController();
+  const state: SharedAutoInstallState = {
+    controller,
+    waiters: 0,
+    settled: false,
+    promise: Promise.resolve({
+      path: RG_BINARY,
+      backend: 'rg' as const,
+      source: 'missing-rg' as const,
+    }),
+  };
 
+  state.promise = (async () => {
     try {
-      const installedPath = await installManagedRipgrep(signal);
+      const installedPath = await installManagedRipgrep(controller.signal);
       return {
         path: installedPath,
         backend: 'rg' as const,
         source: 'managed-rg' as const,
       };
     } catch (error) {
-      if (isAbortLikeError(error) || signal?.aborted) {
+      if (isAbortLikeError(error) || controller.signal.aborted) {
         throw new AbortWaitError(
           'Search was cancelled before execution started.',
         );
@@ -203,13 +240,39 @@ export async function resolveGrepCliWithAutoInstall(
       });
       throw new Error(buildUnavailableBackendMessage(error));
     } finally {
-      autoInstallPromise = null;
+      state.settled = true;
+
+      if (autoInstallState === state) {
+        autoInstallState = null;
+      }
     }
   })();
 
-  return raceWithAbort(autoInstallPromise, signal);
+  return state;
+}
+
+export async function resolveGrepCliWithAutoInstall(
+  deps: GrepResolverDependencies = {},
+  signal?: AbortSignal,
+): Promise<ResolvedGrepCli> {
+  if (signal?.aborted) {
+    throw new AbortWaitError('Search was cancelled before execution started.');
+  }
+
+  const current = resolveSync(deps);
+  if (isResolvedRipgrep(current)) {
+    return current;
+  }
+
+  if (autoInstallState) {
+    return waitForSharedAutoInstall(autoInstallState, signal);
+  }
+
+  autoInstallState = createSharedAutoInstall(deps);
+
+  return waitForSharedAutoInstall(autoInstallState, signal);
 }
 
 export function resetGrepCliResolverForTests(): void {
-  autoInstallPromise = null;
+  autoInstallState = null;
 }