Skip to content

Commit

Permalink
Reduce number of created temp dirs for profiles (closes #2735, closes #…
Browse files Browse the repository at this point in the history
…2013) (#2740)

* Reduce number of created temp dirs for profiles (closes #2735)

* Add test

* Remove temp dirs in a separate process

* Fix tests

* Fix remarks
  • Loading branch information
AndreyBelym authored and miherlosev committed Aug 24, 2018
1 parent 31e1b2c commit 433d608
Show file tree
Hide file tree
Showing 17 changed files with 540 additions and 73 deletions.
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');
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');

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
.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

0 comments on commit 433d608

Please sign in to comment.