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

fix: fast memory/open file handle bail outs #326

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
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
60 changes: 38 additions & 22 deletions src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,16 +84,6 @@ export async function detectElectronPlatform (opts: BaseSignOptions): Promise<El
}
}

/**
* This function returns a promise resolving the file path if file binary.
*/
async function getFilePathIfBinary (filePath: string) {
if (await isBinaryFile(filePath)) {
return filePath;
}
return null;
}

/**
* This function returns a promise validating opts.app, the application to be signed or flattened.
*/
Expand Down Expand Up @@ -136,36 +126,62 @@ export async function validateOptsPlatform (opts: BaseSignOptions): Promise<Elec
export async function walkAsync (dirPath: string): Promise<string[]> {
debugLog('Walking... ' + dirPath);

async function _walkAsync (dirPath: string): Promise<DeepList<string>> {
async function _walkAsync (dirPath: string): Promise<string[]> {
const children = await fs.readdir(dirPath);
return await Promise.all(

const binaryFiles: string[] = [];
const filesToCheck: string[] = [];
const filesToRemove: string[] = [];
const foldersToCheck: string[] = [];

await Promise.all(
children.map(async (child) => {
const filePath = path.resolve(dirPath, child);

const stat = await fs.stat(filePath);
if (stat.isFile()) {
switch (path.extname(filePath)) {
case '.cstemp': // Temporary file generated from past codesign
debugLog('Removing... ' + filePath);
await fs.remove(filePath);
return null;
filesToRemove.push(filePath);
break;
default:
return await getFilePathIfBinary(filePath);
filesToCheck.push(filePath);
}
} else if (stat.isDirectory() && !stat.isSymbolicLink()) {
const walkResult = await _walkAsync(filePath);
foldersToCheck.push(filePath);

switch (path.extname(filePath)) {
case '.app': // Application
case '.framework': // Framework
walkResult.push(filePath);
binaryFiles.push(filePath);
}
return walkResult;
}
return null;
})
);

// Remove all old tmp files -> should be not much per folder recursion
await Promise.all(filesToRemove.map(async (filePath) => {
debugLog(`Removing... ${filePath}`);
await fs.remove(filePath);
}));

// Only binaries need to be signed
// isBinaryFile method opens file, calls stat and alloc 1KB memory and reads in file
// build chunks of 10 files to avoid reaching memory or open file handle limits
const chunkSize = 10;
for (let index = 0; index < filesToCheck.length; index += chunkSize) {
await Promise.all(filesToCheck.slice(index, index + chunkSize).map(
async (filePath) => await isBinaryFile(filePath) && binaryFiles.push(filePath))
);
}

// Do avoid fast and easy memory or open file handle bail outs trigger recursion not in parallel
for (const folderPath of foldersToCheck) {
binaryFiles.push(...(await _walkAsync(folderPath)));
}

return binaryFiles;
}

const allPaths = await _walkAsync(dirPath);
return compactFlattenedList(allPaths);
return await _walkAsync(dirPath);
}