mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-01 06:24:41 +01:00
fix(test:free): a browser record removed mid-read by a shutting-down daemon is absent
Run 13 (feaa28d) needed the flaky-retry for 'owned detached browser
settlement > cancel': 2 unattributed failures instead of 1. A 16-vCPU
stress run reproduced it once in 64 with the cause:
'browser ownership unavailable'. On SIGINT the cancelled daemon removes its
state file; record() saw it exist, then lstat/read hit ENOENT, which
failure() reports as an ownership error. ENOENT there now means absent.
New mode cancel-vanishing-record forces the removal between the check and
the read (pre-fix: 2 unattributed, deterministic; now 1).
This commit is contained in:
1 parent
1597dcca17
commit
c219d8e274
2 files changed
+20
-4
No files matched your search
@@ -1543,12 +1543,12 @@ function trackShardBrowser(stateDir: string, env: NodeJS.ProcessEnv) {
|
||||
}
|
||||
return true;
|
||||
};
|
||||
const record = (file: string): any => {
|
||||
const record = (file: string): any => { try { // a record a shutting-down daemon removes mid-read is absent
|
||||
if (!fs.existsSync(file)) return null;
|
||||
const info = fs.lstatSync(file);
|
||||
if (!info.isFile() || info.size > 65536) throw new BrowserCleanupError('unsafe browser state record');
|
||||
return JSON.parse(fs.readFileSync(file, 'utf8'));
|
||||
};
|
||||
} catch (error) { if ((error as NodeJS.ErrnoException).code === 'ENOENT') return null; throw error; } };
|
||||
const nativeStart = async (capture: Capture, identity: Identity): Promise<string> => {
|
||||
check(capture);
|
||||
const key = `${identity.pid}:${identity.start}`;
|
||||
|
||||
@@ -150,6 +150,19 @@ if (role === 'leaf') {
|
||||
}
|
||||
return value;
|
||||
});
|
||||
} else if (mode === 'cancel-vanishing-record') {
|
||||
// The daemon removes its state file on SIGINT: one existence check sees it,
|
||||
// and it is gone before the record is read.
|
||||
const original = fs.existsSync;
|
||||
spy = spyOn(fs, 'existsSync').mockImplementation((filename) => {
|
||||
if (original(file('.interrupted')) && original(file('.ready')) && !original(file('.vanished'))
|
||||
&& String(filename) === JSON.parse(fs.readFileSync(file('.ready'), 'utf8')).stateFile) {
|
||||
fs.rmSync(String(filename), {force:true});
|
||||
fs.writeFileSync(file('.vanished'), 'true');
|
||||
return true;
|
||||
}
|
||||
return original(filename);
|
||||
});
|
||||
} else if (mode === 'directory-remove-failure') {
|
||||
const original = fs.rmSync;
|
||||
spy = spyOn(fs, 'rmSync').mockImplementation((filename, ...args) => {
|
||||
@@ -188,8 +201,8 @@ function ownershipProcessAlive(identity: { pid: number; start: string; ticks: st
|
||||
describe('test-free-shards: owned detached browser settlement', () => {
|
||||
for (const mode of ['success', 'failure', 'timeout', 'cancel', 'cancel-force', 'endpoint-mix', 'full-state-mix',
|
||||
'terminal-mix', 'chromium-mix', 'replaced-pid', 'stale-child-start', 'replaced-start', 'exit-environment-race',
|
||||
'unavailable-environment', 'directory-remove-failure', 'cancel-cold-probes', 'settle-cold-probes', 'settle-slow-exit']) {
|
||||
test.skipIf(process.platform === 'win32' || ((['replaced-start', 'exit-environment-race', 'unavailable-environment', 'settle-slow-exit'].includes(mode) || mode.endsWith('cold-probes')) && process.platform !== 'linux'))(mode, async () => {
|
||||
'unavailable-environment', 'directory-remove-failure', 'cancel-cold-probes', 'settle-cold-probes', 'settle-slow-exit', 'cancel-vanishing-record']) {
|
||||
test.skipIf(process.platform === 'win32' || ((['replaced-start', 'exit-environment-race', 'unavailable-environment', 'settle-slow-exit', 'cancel-vanishing-record'].includes(mode) || mode.endsWith('cold-probes')) && process.platform !== 'linux'))(mode, async () => {
|
||||
const directory = fs.mkdtempSync(path.join(os.tmpdir(), 'free-owned-browser-'));
|
||||
const actor = path.join(directory, 'actor.ts');
|
||||
fs.writeFileSync(actor, OWNERSHIP_ACTOR);
|
||||
@@ -253,6 +266,9 @@ process.exit(code);
|
||||
'unavailable-environment', 'directory-remove-failure', 'settle-cold-probes'].includes(mode);
|
||||
expect(result.status).toBe(mode === 'timeout' ? 'timed-out' : invalid || mode === 'failure' || mode.startsWith('cancel') ? 'failed' : 'passed');
|
||||
expect(result.unattributedFailures).toBe(invalid || mode === 'timeout' || mode.startsWith('cancel') ? 1 : 0);
|
||||
// Run 13 (feaa28d) and a 16-vCPU stress run: the cancelled daemon's state file
|
||||
// vanished between the existence check and the read ('browser ownership unavailable').
|
||||
if (mode === 'cancel-vanishing-record') expect(fs.existsSync(path.join(directory, 'owned.vanished'))).toBe(true);
|
||||
if (invalid) {
|
||||
expect(fs.existsSync(path.dirname(own.stateFile))).toBe(true);
|
||||
expect(fs.existsSync(path.join(directory, 'owned.interrupted'))).toBe(mode === 'directory-remove-failure');
|
||||
|
||||
Reference in new issue
Block a user