Fix fixed Browser debugger port publication race

libraries/testing_runtime/bin/fkst-fixed-browser-worker.js

@@ -11,6 +11,23 @@ const store = require('../../../examples/generic-host/bin/durable-host-store');
 
 const delay = ms => new Promise(resolve => setTimeout(resolve, ms));
 
+async function waitForDebuggerPort(portFile, deadline, isClosed) {
+  while (true) {
+    check(!isClosed() && Date.now() < deadline, 'chrome-start-failed');
+    let contents;
+    try { contents = fs.readFileSync(portFile, 'utf8'); }
+    catch (error) { if (error.code !== 'ENOENT') throw error; }
+    // Chromium creates the file before writing it. A port prefix is not ready
+    // until its terminating newline has been published.
+    if (contents !== undefined && contents.includes('\n')) {
+      const port = contents.split('\n')[0];
+      check(/^[1-9][0-9]{0,4}$/.test(port) && Number(port) <= 65535, 'debugger-port-invalid');
+      return port;
+    }
+    await delay(25);
+  }
+}
+
 async function execute(input) {
   const { plan, policy, root, execution_id: id, profile } = input;
   let chrome; let cdp; let deadlineTimer; let closed = false; let profileIdentity;
@@ -53,11 +70,7 @@ async function execute(input) {
     chrome.stdin.end('start\n');
     deadlineTimer = setTimeout(() => { chrome.kill('SIGKILL'); }, plan.timeout_ms);
     const portFile = path.join(profile, 'DevToolsActivePort');
-    while (!fs.existsSync(portFile)) {
-      check(!closed && !expired(), 'chrome-start-failed'); await delay(25);
-    }
-    const port = fs.readFileSync(portFile, 'utf8').split('\n')[0];
-    check(/^[1-9][0-9]{0,4}$/.test(port), 'debugger-port-invalid');
+    const port = await waitForDebuggerPort(portFile, deadline, () => closed);
     const response = await fetch(`http://127.0.0.1:${port}/json/new?about:blank`, {
       method: 'PUT', signal: AbortSignal.timeout(Math.max(1, deadline - Date.now())),
     });
@@ -162,3 +175,5 @@ if (require.main === module) {
     process.stdout.write(JSON.stringify(result) + '\n');
   }).catch(() => { process.exitCode = 1; }));
 }
+
+module.exports = { waitForDebuggerPort };

libraries/testing_runtime/tests/fixed_browser_test.js

@@ -11,6 +11,7 @@ const fixed = require('../lib/deterministic_browser');
 const { context } = require('../../../examples/generic-host/bin/deterministic-browser');
 const store = require('../../../examples/generic-host/bin/durable-host-store');
 const candidateFixture = require('./fixtures/browser-host-candidate.json');
+const { waitForDebuggerPort } = require('../bin/fkst-fixed-browser-worker');
 
 const ROOT = path.resolve(__dirname, '../../..');
 const CLI = path.join(ROOT, 'examples/generic-host/bin/deterministic-browser.js');
@@ -62,6 +63,58 @@ function fakeHost(value) {
   return { host, effects: () => effects };
 }
 
+test('debugger readiness waits for creation, empty file and partial port publication', async t => {
+  const portFile = '/fixture/DevToolsActivePort';
+  const states = [null, '', '12', '12345\n/devtools/browser/fixture'];
+  const readFileSync = fs.readFileSync;
+  let reads = 0;
+  t.mock.method(fs, 'readFileSync', (file, ...args) => {
+    if (file !== portFile) return readFileSync(file, ...args);
+    assert.ok(reads < states.length);
+    const value = states[reads++];
+    if (value === null) throw Object.assign(new Error('not yet created'), { code: 'ENOENT' });
+    return value;
+  });
+  assert.equal(await waitForDebuggerPort(portFile, Date.now() + 2000, () => false), '12345');
+  assert.equal(reads, states.length);
+});
+
+test('debugger readiness rejects a completed invalid port and non-ENOENT IO failure', async t => {
+  const portFile = '/fixture/DevToolsActivePort';
+  const readFileSync = fs.readFileSync;
+  let contents;
+  t.mock.method(fs, 'readFileSync', (file, ...args) => {
+    if (file !== portFile) return readFileSync(file, ...args);
+    if (contents instanceof Error) throw contents;
+    return contents;
+  });
+  for (contents of ['\n', '0\n', '65536\n', '123456\n', '12x\n', '123\r\n']) {
+    await assert.rejects(waitForDebuggerPort(portFile, Date.now() + 2000, () => false), /debugger-port-invalid/);
+  }
+  contents = Object.assign(new Error('denied'), { code: 'EACCES' });
+  await assert.rejects(waitForDebuggerPort(portFile, Date.now() + 2000, () => false), { code: 'EACCES' });
+});
+
+test('debugger readiness stays bounded for an incomplete record or exited Chrome', async t => {
+  const portFile = '/fixture/DevToolsActivePort';
+  const readFileSync = fs.readFileSync;
+  let reads = 0;
+  let closed = false;
+  let now = 0;
+  t.mock.method(Date, 'now', () => now);
+  t.mock.method(fs, 'readFileSync', (file, ...args) => {
+    if (file !== portFile) return readFileSync(file, ...args);
+    reads += 1;
+    if (reads === 1) now = 100;
+    else closed = true;
+    return '12';
+  });
+  await assert.rejects(waitForDebuggerPort(portFile, 100, () => closed), /chrome-start-failed/);
+  assert.equal(reads, 1);
+  await assert.rejects(waitForDebuggerPort(portFile, 200, () => closed), /chrome-start-failed/);
+  assert.equal(reads, 2);
+});
+
 test('compile exact original PQL staged candidate; deterministic and nonauthorizing', t => {
   const { candidate, policy, plan } = inputs(t);
   assert.deepEqual(fixed.compile(candidate, policy), plan);