embarklabs/embark
2
0
Fork 0
mirror of https://github.com/embarklabs/embark.git synced 2025-02-22 10:28:34 +00:00
Code Issues Projects Releases Wiki Activity

fix(@embark/core): fix memory leak when contract files are loaded

Browse Source 
Prior to this commits `this.contractsFiles` kept growing at run time
for every file change that was detected by Embark.

The reason for that was a bug in a condition that determines whether
a subset of contract files needed to be added to memory or not.

In addition to that we kept pushing externally loaded contract files into
memory, even if they already existed in memory.
This commit is contained in:
Pascal Precht 2019-02-07 16:31:45 +01:00 committed by Iuri Matias
parent 2e763f3720
commit 40b9ac3c70
4 changed files with 46 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

57
packages/embark/src/lib/core/config.js
View File

@ -59,7 +59,7 @@ var Config = function(options) {
resolver = resolver || function(callback) {
callback(fs.readFileSync(filename).toString());
};
self.contractsFiles.push(new File({path: filename, type: Types.custom, resolver}));
self.contractsFiles.push(new File({path: filename, originalPath: filename, type: Types.custom, resolver}));
});
self.events.on('file-remove', (fileType, removedPath) => {
@ -120,15 +120,16 @@ Config.prototype.reloadConfig = function() {
};
Config.prototype.loadContractFiles = function() {
const contracts = this.embarkConfig.contracts;
const newContractsFiles = this.loadFiles(contracts);
if (!this.contractFiles || newContractsFiles.length !== this.contractFiles.length || !deepEqual(newContractsFiles, this.contractFiles)) {
this.contractsFiles = this.contractsFiles.concat(newContractsFiles).filter((file, index, arr) => {
return !arr.some((file2, index2) => {
return file.path === file2.path && index < index2;
});
});
}
const loadedContractFiles = this.loadFiles(this.embarkConfig.contracts);
// `this.contractsFiles` could've been mutated at runtime using
// either `config:contractsFiles:add` event or through calls to
// `loadExternalContractsFiles()`, so we have to make sure we preserve
// those added files before we reset `this.contractsFiles`.
//
// We do that by determining the difference between `loadedContractFiles` and the ones
// already in memory in `this.contractsFiles`.
const addedContractFiles = this.contractsFiles.filter(existingFile => !loadedContractFiles.some(file => file.originalPath === existingFile.originalPath));
this.contractsFiles = loadedContractFiles.concat(addedContractFiles);
};
Config.prototype._updateBlockchainCors = function(){
@ -358,20 +359,36 @@ Config.prototype.loadExternalContractsFiles = function() {
}
for (let contractName in contracts) {
let contract = contracts[contractName];
if (!contract.file) {
continue;
}
let externalContractFile = null;
if (contract.file.startsWith('http') || contract.file.startsWith('git') || contract.file.startsWith('ipfs') || contract.file.startsWith('bzz')) {
const fileObj = utils.getExternalContractUrl(contract.file,this.providerUrl);
const fileObj = utils.getExternalContractUrl(contract.file, this.providerUrl);
if (!fileObj) {
return this.logger.error(__("HTTP contract file not found") + ": " + contract.file);
}
const localFile = fileObj.filePath;
this.contractsFiles.push(new File({path: localFile, type: Types.http, basedir: '', externalUrl: fileObj.url, storageConfig: storageConfig}));
externalContractFile = new File({ path: fileObj.filePath, originalPath: fileObj.filePath, type: Types.http, basedir: '', externalUrl: fileObj.url, storageConfig });
} else if (fs.existsSync(contract.file)) {
this.contractsFiles.push(new File({path: contract.file, type: Types.dappFile, basedir: '', storageConfig: storageConfig}));
externalContractFile = new File({ path: contract.file, originalPath: contract.file, type: Types.dappFile, basedir: '', storageConfig });
} else if (fs.existsSync(path.join('./node_modules/', contract.file))) {
this.contractsFiles.push(new File({path: path.join('./node_modules/', contract.file), type: Types.dappFile, basedir: '', storageConfig: storageConfig}));
const completePath = path.join('./node_modules/', contract.file);
externalContractFile = new File({ path: completePath, originalPath: completePath, type: Types.dappFile, basedir: '', storageConfig });
}
if (externalContractFile) {
const index = this.contractsFiles.findIndex(contractFile => contractFile.originalPath === externalContractFile.originalPath);
// It's important that we only add `externalContractFile` if it doesn't exist already
// within `contractsFiles`, otherwise we keep adding duplicates in subsequent
// compilation routines creating a memory leak.
if (index > -1) {
this.contractsFiles[index] = externalContractFile;
} else {
this.contractsFiles.push(externalContractFile);
}
} else {
this.logger.error(__("contract file not found") + ": " + contract.file);
}
@ -563,7 +580,7 @@ Config.prototype.loadFiles = function(files) {
return (file[0] === '$' || file.indexOf('.') >= 0);
}).filter(function(file) {
let basedir = findMatchingExpression(file, files);
readFiles.push(new File({path: file, type: Types.dappFile, basedir: basedir, storageConfig: storageConfig}));
readFiles.push(new File({path: file, originalPath: file, type: Types.dappFile, basedir: basedir, storageConfig: storageConfig}));
});
var filesFromPlugins = [];
@ -597,9 +614,11 @@ Config.prototype.loadPluginContractFiles = function() {
contractsPlugins.forEach(function(plugin) {
plugin.contractsFiles.forEach(function(file) {
var filename = file.replace('./','');
self.contractsFiles.push(new File({path: filename, pluginPath: plugin.pluginPath, type: Types.custom, storageConfig: storageConfig, resolver: function(callback) {
callback(plugin.loadPluginFile(file));
}}));
self.contractsFiles.push(new File({ path: filename, originalPath: path.join(plugin.pluginPath, filename), pluginPath: plugin.pluginPath, type: Types.custom, storageConfig,
resolver: function(callback) {
callback(plugin.loadPluginFile(file));
}
}));
});
});
};

2
packages/embark/src/lib/core/file.ts
View File

@ -25,6 +25,7 @@ export class File {
public storageConfig: any;
public providerUrl: string;
public importRemappings: ImportRemapping[] = [];
public originalPath: string;
constructor(options: any) {
this.type = options.type;
@ -34,6 +35,7 @@ export class File {
this.pluginPath = options.pluginPath ? options.pluginPath : "";
this.storageConfig = options.storageConfig;
this.providerUrl = "";
this.originalPath = options.originalPath || "";
if (this.type === Types.http) {
const external = utils.getExternalContractUrl(options.externalUrl, this.providerUrl);

6
packages/embark/src/lib/utils/solidity/remapImports.ts
View File

@ -65,7 +65,7 @@ const buildNewFile = (file: File, importPath: string) => {
from = resolve(importPath);
to = fs.dappPath(".embark", "node_modules", importPath);
fs.copySync(from, to);
return new File({ path: to, type: Types.dappFile });
return new File({ path: to, type: Types.dappFile, originalPath: from });
}
// started with node_modules then further imports local paths in it's own repo/directory
@ -73,7 +73,7 @@ const buildNewFile = (file: File, importPath: string) => {
from = path.join(path.dirname(file.path.replace(".embark", ".")), importPath);
to = path.join(path.dirname(file.path), importPath);
fs.copySync(from, to);
return new File({ path: to, type: Types.dappFile });
return new File({ path: to, type: Types.dappFile, originalPath: from });
}
// local import, ie import "../path/to/contract" or "./path/to/contract"
@ -81,7 +81,7 @@ const buildNewFile = (file: File, importPath: string) => {
to = path.join(path.dirname(file.path), importPath);
fs.copySync(from, to);
return new File({ path: to, type: Types.dappFile });
return new File({ path: to, type: Types.dappFile, originalPath: from });
};
const rescursivelyFindRemapImports = async (file: File, filesProcessed: string[] = []) => {

3
packages/embark/src/test/config.js
View File

@ -192,6 +192,7 @@ describe('embark.Config', function () {
"type": "http",
"externalUrl": "https://raw.githubusercontent.com/embark-framework/embark/master/test_dapps/packages/test_app/app/contracts/simple_storage.sol",
"path": ".embark/contracts/embark-framework/embark/master/test_dapps/packages/test_app/app/contracts/simple_storage.sol",
"originalPath": ".embark/contracts/embark-framework/embark/master/test_dapps/packages/test_app/app/contracts/simple_storage.sol",
"pluginPath": '',
"basedir": "",
"importRemappings": [],
@ -203,6 +204,7 @@ describe('embark.Config', function () {
"type": "http",
"externalUrl": "https://raw.githubusercontent.com/status-im/contracts/master/contracts/identity/ERC725.sol",
"path": ".embark/contracts/status-im/contracts/master/contracts/identity/ERC725.sol",
"originalPath": ".embark/contracts/status-im/contracts/master/contracts/identity/ERC725.sol",
"pluginPath": '',
"basedir": "",
"importRemappings": [],
@ -213,6 +215,7 @@ describe('embark.Config', function () {
{
"externalUrl": "https://swarm-gateways.net/bzz:/1ffe993abc835f480f688d07ad75ad1dbdbd1ddb368a08b7ed4d3e400771dd63",
"path": ".embark/contracts/bzz:/1ffe993abc835f480f688d07ad75ad1dbdbd1ddb368a08b7ed4d3e400771dd63",
"originalPath": ".embark/contracts/bzz:/1ffe993abc835f480f688d07ad75ad1dbdbd1ddb368a08b7ed4d3e400771dd63",
"type": "http",
"pluginPath": '',
"basedir": "",