Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce number of created temp dirs for profiles (closes #2735, closes #2013) #2740

Merged
merged 5 commits into from
Aug 24, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions src/browser/provider/built-in/chrome/create-temp-profile.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
import path from 'path';
import tmp from 'tmp';
import TempDirectory from '../../../../utils/temp-directory';
import { writeFile, ensureDir } from '../../../../utils/promisified-functions';


export default async function (proxyHostName) {
tmp.setGracefulCleanup();

const tempDir = tmp.dirSync({ unsafeCleanup: true });
const profileDirName = path.join(tempDir.name, 'Default');
const tempDir = await TempDirectory.createDirectory('chrome-profile');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be testcafe-chrome-profile to avoid the name conflicts.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solved in favor of top-level testcafe-tmp.

const profileDirName = path.join(tempDir.path, 'Default');

await ensureDir(profileDirName);

Expand Down Expand Up @@ -39,7 +37,7 @@ export default async function (proxyHostName) {
};

await writeFile(path.join(profileDirName, 'Preferences'), JSON.stringify(preferences));
await writeFile(path.join(tempDir.name, 'First Run'), '');
await writeFile(path.join(tempDir.path, 'First Run'), '');

return tempDir;
}
3 changes: 3 additions & 0 deletions src/browser/provider/built-in/chrome/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ export default {
if (OS.mac || runtimeInfo.config.headless)
await stopLocalChrome(runtimeInfo);

if (runtimeInfo.tempProfileDir)
await runtimeInfo.tempProfileDir.dispose();

delete this.openedBrowsers[browserId];
},

Expand Down
4 changes: 2 additions & 2 deletions src/browser/provider/built-in/chrome/local-chrome.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import browserTools from 'testcafe-browser-tools';
import killBrowserProcess from '../../utils/kill-browser-process';
import killBrowserProcess from '../../../../utils/kill-browser-process';
import BrowserStarter from '../../utils/browser-starter';


Expand All @@ -9,7 +9,7 @@ function buildChromeArgs (config, cdpPort, platformArgs, profileDir) {
return []
.concat(
cdpPort ? [`--remote-debugging-port=${cdpPort}`] : [],
!config.userProfile ? [`--user-data-dir=${profileDir.name}`] : [],
!config.userProfile ? [`--user-data-dir=${profileDir.path}`] : [],
config.headless ? ['--headless'] : [],
config.userArgs ? [config.userArgs] : [],
platformArgs ? [platformArgs] : []
Expand Down
22 changes: 3 additions & 19 deletions src/browser/provider/built-in/chrome/runtime-info.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,10 @@ import getConfig from './config';
import createTempProfile from './create-temp-profile';


var commonTempProfile = null;

async function getTempProfile (proxyHostName, config) {

var tempProfile = commonTempProfile;
var shouldUseCommonProfile = !config.headless && !config.emulation;

if (!shouldUseCommonProfile || !commonTempProfile)
tempProfile = await createTempProfile(proxyHostName);

if (shouldUseCommonProfile && !commonTempProfile)
commonTempProfile = tempProfile;

return tempProfile;
}

export default async function (proxyHostName, configString) {
var config = getConfig(configString);
var tempProfileDir = !config.userProfile ? await getTempProfile(proxyHostName, config) : null;
var cdpPort = config.cdpPort || (!config.userProfile ? await getFreePort() : null);
const config = getConfig(configString);
const tempProfileDir = !config.userProfile ? await createTempProfile(proxyHostName, config) : null;
const cdpPort = config.cdpPort || (!config.userProfile ? await getFreePort() : null);

return { config, cdpPort, tempProfileDir };
}
8 changes: 3 additions & 5 deletions src/browser/provider/built-in/firefox/create-temp-profile.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import path from 'path';
import tmp from 'tmp';
import TempDirectory from '../../../../utils/temp-directory';
import { writeFile } from '../../../../utils/promisified-functions';


Expand Down Expand Up @@ -54,11 +54,9 @@ async function generatePreferences (profileDir, { marionettePort, config }) {
}

export default async function (runtimeInfo) {
tmp.setGracefulCleanup();
const tmpDir = await TempDirectory.createDirectory('firefox-profile');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

testcafe-firefox-profile

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solved in favor of top-level testcafe-tmp.


const tmpDir = tmp.dirSync({ unsafeCleanup: true });

await generatePreferences(tmpDir.name, runtimeInfo);
await generatePreferences(tmpDir.path, runtimeInfo);

return tmpDir;
}
3 changes: 3 additions & 0 deletions src/browser/provider/built-in/firefox/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ export default {
if (OS.mac && !config.headless)
await stopLocalFirefox(runtimeInfo);

if (runtimeInfo.tempProfileDir)
await runtimeInfo.tempProfileDir.dispose();

delete this.openedBrowsers[browserId];
},

Expand Down
4 changes: 2 additions & 2 deletions src/browser/provider/built-in/firefox/local-firefox.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import OS from 'os-family';
import browserTools from 'testcafe-browser-tools';
import killBrowserProcess from '../../utils/kill-browser-process';
import killBrowserProcess from '../../../../utils/kill-browser-process';
import BrowserStarter from '../../utils/browser-starter';


Expand All @@ -18,7 +18,7 @@ function buildFirefoxArgs (config, platformArgs, { marionettePort, tempProfileDi
return []
.concat(
marionettePort ? ['-marionette'] : [],
!config.userProfile ? ['-no-remote', '-new-instance', `-profile "${tempProfileDir.name}"`] : [],
!config.userProfile ? ['-no-remote', '-new-instance', `-profile "${tempProfileDir.path}"`] : [],
config.headless ? ['-headless'] : [],
config.userArgs ? [config.userArgs] : [],
platformArgs ? [platformArgs] : []
Expand Down
38 changes: 0 additions & 38 deletions src/browser/provider/utils/kill-browser-process.js

This file was deleted.

63 changes: 63 additions & 0 deletions src/utils/kill-browser-process.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import { spawn } from 'child_process';
import OS from 'os-family';
import promisifyEvent from 'promisify-event';
import Promise from 'pinkie';
import { findProcess, killProcess } from './promisified-functions';


const BROWSER_CLOSING_TIMEOUT = 5;

async function runWMIC (args) {
const wmicProcess = spawn('wmic.exe', args, { detached: true });

let wmicOutput = '';

wmicProcess.stdout.on('data', data => {
wmicOutput += data.toString();
});

try {
await Promise.race([
promisifyEvent(wmicProcess.stdout, 'end'),
promisifyEvent(wmicProcess, 'error')
]);

return wmicOutput;
}
catch (e) {
return '';
}
}

async function findProcessWin (processOptions) {
var wmicArgs = ['process', 'where', `commandline like '%${processOptions.arguments}%' and name <> 'cmd.exe' and name <> 'wmic.exe'`, 'get', 'processid'];
var wmicOutput = await runWMIC(wmicArgs);
var processList = wmicOutput.split(/\s*\n/);

processList = processList
// NOTE: remove list's header and empty last element, caused by trailing newline
.slice(1, -1)
.map(pid => ({ pid: Number(pid) }));

return processList;
}

export default async function (browserId) {
var processOptions = { arguments: browserId, psargs: '-ef' };
var processList = OS.win ? await findProcessWin(processOptions) : await findProcess(processOptions);

if (!processList.length)
return true;

try {
if (OS.win)
process.kill(processList[0].pid);
else
await killProcess(processList[0].pid, { timeout: BROWSER_CLOSING_TIMEOUT });

return true;
}
catch (e) {
return false;
}
}
3 changes: 3 additions & 0 deletions src/utils/promisified-functions.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import promisify from './promisify';


export const ensureDir = promisify(mkdirp);
export const readDir = promisify(fs.readdir);
export const stat = promisify(fs.stat);
export const writeFile = promisify(fs.writeFile);
export const readFile = promisify(fs.readFile);
Expand All @@ -15,3 +16,5 @@ export const findProcess = promisify(psNode.lookup);
export const killProcess = promisify(psNode.kill);

export const exec = promisify(childProcess.exec);

export const sendMessageToChildProcess = promisify((process, ...args) => process.send(...args));
5 changes: 5 additions & 0 deletions src/utils/temp-directory/cleanup-process/commands.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export default {
init: 'init',
add: 'add',
remove: 'remove'
};
136 changes: 136 additions & 0 deletions src/utils/temp-directory/cleanup-process/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
import { spawn } from 'child_process';
import debug from 'debug';
import promisifyEvent from 'promisify-event';
import Promise from 'pinkie';
import { sendMessageToChildProcess } from '../../promisified-functions';
import COMMANDS from './commands';


const WORKER_PATH = require.resolve('./worker');
const WORKER_STDIO_CONFIG = ['ignore', 'ignore', 'ignore', 'ipc'];

const DEBUG_LOGGER = debug('testcafe:utils:temp-directory:cleanup-process');

class CleanupProcess {
constructor () {
this.worker = null;
this.initialized = false;
this.initPromise = Promise.resolve(void 0);

this.messageCounter = 0;

this.pendingResponses = {};
}

_sendMessage (id, msg) {
return sendMessageToChildProcess(this.worker, { id, ...msg });
}

_onResponse (response) {
const pendingResponse = this.pendingResponses[response.id];

if (response.error) {
if (pendingResponse)
pendingResponse.control.reject(response.error);
else
this.pendingResponses[response.id] = Promise.reject(response.error);
}
else if (pendingResponse)
pendingResponse.control.resolve();
else
this.pendingResponses[response.id] = Promise.resolve();
}

async _waitResponse (id) {
if (!this.pendingResponses[id]) {
const promiseControl = {};

this.pendingResponses[id] = new Promise((resolve, reject) => {
Object.assign(promiseControl, { resolve, reject });
});

this.pendingResponses[id].control = promiseControl;
}

try {
await this.pendingResponses[id];
}
finally {
delete this.pendingResponses[id];
}
}

async _waitResponseForMessage (msg) {
const currentId = this.messageCounter;

this.messageCounter++;

await this._sendMessage(currentId, msg);
await this._waitResponse(currentId);
}

init () {
this.initPromise = this.initPromise
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does it work? Is it the workaround for the concurrent init?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's a guard that ensures that init code will be run sequentially when called concurrently. This situation can happen when starting both Chrome and Firefox, or in concurrency mode. Calls to init form a Promise chain, where each next call have to wait until all previous calls are resolved.

.then(async initialized => {
if (initialized !== void 0)
return initialized;

this.worker = spawn(process.argv[0], [WORKER_PATH], { detached: true, stdio: WORKER_STDIO_CONFIG });

this.worker.on('message', message => this._onResponse(message));

this.worker.unref();

try {
await Promise.race([
this._waitResponseForMessage({ command: COMMANDS.init }),
promisifyEvent(this.worker, 'error')
]);

const channel = this.worker.channel || this.worker._channel;

channel.unref();

this.initialized = true;
}
catch (e) {
DEBUG_LOGGER('Failed to start cleanup process');
DEBUG_LOGGER(e);

this.initialized = false;
}

return this.initialized;
});

return this.initPromise;
}

async addDirectory (path) {
if (!this.initialized)
return;

try {
await this._waitResponseForMessage({ command: COMMANDS.add, path });
}
catch (e) {
DEBUG_LOGGER(`Failed to add the ${path} directory to cleanup process`);
DEBUG_LOGGER(e);
}
}

async removeDirectory (path) {
if (!this.initialized)
return;

try {
await this._waitResponseForMessage({ command: COMMANDS.remove, path });
}
catch (e) {
DEBUG_LOGGER(`Failed to remove the ${path} directory in cleanup process`);
DEBUG_LOGGER(e);
}
}
}

export default new CleanupProcess();
Loading