fix: scope xcodebuild process kill to this package's own processes

lib/constants.ts

@@ -14,3 +14,12 @@ export const PLATFORM_NAME_IOS = 'iOS';
 export const SDK_DEVICE = 'iphoneos';
 
 export const WDA_UPGRADE_TIMESTAMP_PATH = path.join('.appium', 'webdriveragent', 'upgrade.time');
+
+/**
+ * Harmless unused build setting override appended to every xcodebuild invocation
+ * this package starts. It has no effect on the build itself, but shows up verbatim
+ * in the process' command line, letting us tell our own xcodebuild processes apart
+ * from unrelated ones (e.g. other WDA-based test runners) that happen to target the
+ * same device udid.
+ */
+export const XCODEBUILD_PROCESS_MARKER = 'APPIUM_XCODEBUILD_WDA_MARKER=1';

lib/utils/processes.ts

@@ -1,15 +1,21 @@
 import {waitForCondition} from 'asyncbox';
 import {exec} from 'teen_process';
 
+import {XCODEBUILD_PROCESS_MARKER} from '../constants.js';
 import {log} from '../logger.js';
 
 /**
  * Find and terminate all processes matching the given pgrep pattern.
+ *
+ * @param pgrepPattern - Pattern used to find candidate processes.
+ * @param cmdlineIncludes - If given, a candidate is only killed if its full
+ * command line also contains this substring. Used to narrow a broad pgrep
+ * match (e.g. by device udid) down to processes this package actually started.
  */
-export async function killAppUsingPattern(pgrepPattern: string): Promise<void> {
+export async function killAppUsingPattern(pgrepPattern: string, cmdlineIncludes?: string): Promise<void> {
   const signals = [2, 15, 9];
   for (const signal of signals) {
-    const matchedPids = await getPIDsUsingPattern(pgrepPattern);
+    const matchedPids = await getPIDsUsingPattern(pgrepPattern, cmdlineIncludes);
     if (matchedPids.length === 0) {
       return;
     }
@@ -52,6 +58,12 @@ export async function killAppUsingPattern(pgrepPattern: string): Promise<void> {
 
 /**
  * Kills running XCTest processes for the particular device.
+ *
+ * The `xcodebuild` pattern is additionally scoped to processes this package started
+ * (see {@link XCODEBUILD_PROCESS_MARKER}), so other XCTest-based tools targeting the
+ * same udid (e.g. a separately managed WebDriverAgent instance) are left alone.
+ * The XCTRunner/xctest patterns below cannot be scoped the same way, since those
+ * processes do not inherit xcodebuild's command line.
  */
 export async function resetTestProcesses(udid: string, isSimulator: boolean): Promise<void> {
   const processPatterns = [`xcodebuild.*${udid}`];
@@ -61,7 +73,37 @@ export async function resetTestProcesses(udid: string, isSimulator: boolean): Pr
     processPatterns.push(`xctest.*${udid}`);
   }
   log.debug(`Killing running processes '${processPatterns.join(', ')}' for the device ${udid}...`);
-  await Promise.all(processPatterns.map(killAppUsingPattern));
+  await Promise.all(
+    processPatterns.map((pattern) =>
+      killAppUsingPattern(pattern, pattern.startsWith('xcodebuild') ? XCODEBUILD_PROCESS_MARKER : undefined),
+    ),
+  );
+}
+
+/**
+ * Filters a list of PIDs down to those whose full command line satisfies the
+ * given lambda. PIDs that have already exited are silently dropped.
+ */
+async function filterPIDsByCommandLine(
+  pids: string[],
+  filteringFunc: (cmdline: string) => boolean | Promise<boolean>,
+): Promise<string[]> {
+  const filtered = await Promise.all(
+    pids.map(async (pid) => {
+      let stdout: string;
+      try {
+        ({stdout} = await exec('ps', ['-p', pid, '-o', 'command']));
+      } catch (e: any) {
+        if (e.code === 1) {
+          // The process does not exist anymore, there's nothing to filter
+          return null;
+        }
+        throw e;
+      }
+      return (await filteringFunc(stdout)) ? pid : null;
+    }),
+  );
+  return filtered.filter((pid): pid is string => Boolean(pid));
 }
 
 /**
@@ -97,32 +139,18 @@ export async function getPIDsListeningOnPort(
   if (typeof filteringFunc !== 'function') {
     return result;
   }
-  const filtered = await Promise.all(
-    result.map(async (pid) => {
-      let stdout: string;
-      try {
-        ({stdout} = await exec('ps', ['-p', pid, '-o', 'command']));
-      } catch (e: any) {
-        if (e.code === 1) {
-          // The process does not exist anymore, there's nothing to filter
-          return null;
-        }
-        throw e;
-      }
-      return (await filteringFunc(stdout)) ? pid : null;
-    }),
-  );
-  return filtered.filter((pid): pid is string => Boolean(pid));
+  return await filterPIDsByCommandLine(result, filteringFunc);
 }
 
-async function getPIDsUsingPattern(pattern: string): Promise<string[]> {
+async function getPIDsUsingPattern(pattern: string, cmdlineIncludes?: string): Promise<string[]> {
   const args = [
     '-if', // case insensitive, full cmdline match
     pattern,
   ];
+  let pids: string[];
   try {
     const {stdout} = await exec('pgrep', args);
-    return stdout
+    pids = stdout
       .split(/\s+/)
       .map((x) => parseInt(x, 10))
       .filter(Number.isInteger)
@@ -131,4 +159,8 @@ async function getPIDsUsingPattern(pattern: string): Promise<string[]> {
     log.debug(`'pgrep ${args.join(' ')}' didn't detect any matching processes. Return code: ${err.code}`);
     return [];
   }
+  if (!cmdlineIncludes || pids.length === 0) {
+    return pids;
+  }
+  return await filterPIDsByCommandLine(pids, (cmdline) => cmdline.includes(cmdlineIncludes));
 }

lib/xcodebuild.ts

@@ -5,7 +5,7 @@ import type {AppiumLogger, StringRecord} from '@appium/types';
 import {retryInterval} from 'asyncbox';
 import {SubProcess, exec} from 'teen_process';
 
-import {WDA_RUNNER_BUNDLE_ID} from './constants.js';
+import {WDA_RUNNER_BUNDLE_ID, XCODEBUILD_PROCESS_MARKER} from './constants.js';
 import {log as defaultLogger} from './logger.js';
 import type {NoSessionProxy} from './no-session-proxy.js';
 import type {
@@ -431,6 +431,10 @@ export class XcodeBuild {
     // with preventing to generate `/Index/DataStore` which is used by development
     args.push('COMPILER_INDEX_STORE_ENABLE=NO');
 
+    // Tags this process so resetTestProcesses() can kill only xcodebuild instances
+    // this package started, not unrelated ones sharing the same device udid.
+    args.push(XCODEBUILD_PROCESS_MARKER);
+
     return {cmd, args};
   }
 

test/unit/processes.spec.ts

@@ -0,0 +1,99 @@
+import assert from 'node:assert/strict';
+import {describe, beforeEach, it, mock} from 'node:test';
+
+interface ExecCall {
+  cmd: string;
+  args: string[];
+}
+
+let pgrepStdout = '';
+let cmdlineByPid: Record<string, string> = {};
+let killedPids: string[] = [];
+const execCalls: ExecCall[] = [];
+
+async function fakeExec(cmd: string, args: string[] = []): Promise<{stdout: string}> {
+  execCalls.push({cmd, args});
+  if (cmd === 'pgrep') {
+    return {stdout: pgrepStdout};
+  }
+  if (cmd === 'ps') {
+    const pid = args[args.indexOf('-p') + 1];
+    return {stdout: cmdlineByPid[pid] ?? ''};
+  }
+  if (cmd === 'kill') {
+    if (args[0] === '-0') {
+      // Report the process as already gone, so killAppUsingPattern does not
+      // wait out the full polling window on every signal.
+      throw Object.assign(new Error('No such process'), {code: 1});
+    }
+    killedPids.push(...args.filter((a) => !a.startsWith('-')));
+    return {stdout: ''};
+  }
+  throw new Error(`Unexpected exec call: ${cmd} ${args.join(' ')}`);
+}
+
+mock.module('teen_process', {
+  namedExports: {
+    exec: (...args: [string, string[]?]) => fakeExec(...args),
+  },
+});
+
+const {killAppUsingPattern, resetTestProcesses} = await import('../../lib/utils/processes.js');
+const {XCODEBUILD_PROCESS_MARKER} = await import('../../lib/constants.js');
+
+describe('processes', function () {
+  beforeEach(function () {
+    pgrepStdout = '';
+    cmdlineByPid = {};
+    killedPids = [];
+    execCalls.length = 0;
+  });
+
+  describe('#killAppUsingPattern', function () {
+    it('kills every matched pid when no cmdline filter is given', async function () {
+      pgrepStdout = '111 222';
+      await killAppUsingPattern('xcodebuild.*some-udid');
+      assert.deepStrictEqual(killedPids.sort(), ['111', '222']);
+    });
+
+    it('only kills pids whose full command line contains the given substring', async function () {
+      pgrepStdout = '111 222';
+      cmdlineByPid = {
+        111: `xcodebuild -destination id=some-udid ${XCODEBUILD_PROCESS_MARKER}`,
+        222: 'xcodebuild -destination id=some-udid', // unrelated xcodebuild instance, no marker
+      };
+      await killAppUsingPattern('xcodebuild.*some-udid', XCODEBUILD_PROCESS_MARKER);
+      assert.deepStrictEqual(killedPids, ['111']);
+    });
+
+    it('kills nothing when no matched pid contains the required substring', async function () {
+      pgrepStdout = '222';
+      cmdlineByPid = {
+        222: 'xcodebuild -destination id=some-udid',
+      };
+      await killAppUsingPattern('xcodebuild.*some-udid', XCODEBUILD_PROCESS_MARKER);
+      assert.deepStrictEqual(killedPids, []);
+    });
+  });
+
+  describe('#resetTestProcesses', function () {
+    it('scopes the xcodebuild pattern to this package own processes on a real device', async function () {
+      pgrepStdout = '111 222';
+      cmdlineByPid = {
+        111: `xcodebuild -destination id=some-udid ${XCODEBUILD_PROCESS_MARKER}`,
+        222: 'xcodebuild -destination id=some-udid', // e.g. a separately managed WDA instance
+      };
+      await resetTestProcesses('some-udid', false);
+      assert.deepStrictEqual(killedPids, ['111']);
+    });
+
+    it('does not apply the marker filter to the simulator XCTRunner/xctest patterns', async function () {
+      pgrepStdout = '333';
+      cmdlineByPid = {
+        333: 'some-path/XCTRunner some-udid', // no marker present, unlike the xcodebuild process
+      };
+      await resetTestProcesses('some-udid', true);
+      assert.ok(killedPids.includes('333'));
+    });
+  });
+});