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: avoid orphaning chained and "other-queue" callbacks #46

Merged
merged 3 commits into from
Nov 10, 2022
Merged
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
40 changes: 20 additions & 20 deletions lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,12 @@ export enum ProcessDataFlag {
CommandLine = 2
}

interface IRequest<T> {
callback: (result: T) => void;
rootPid: number;
}

type RequestQueue<T> = IRequest<T>[];
type RequestCallback = (processList: IProcessInfo[]) => void;

// requestInProgress is used for any function that uses CreateToolhelp32Snapshot, as multiple calls
// to this cannot be done at the same time.
let requestInProgress = false;
const processListRequestQueue: RequestQueue<IProcessInfo[] | undefined> = [];
const processTreeRequestQueue: RequestQueue<IProcessTreeNode | undefined> = [];
const globalRequestQueue: RequestCallback[] = [];

const MAX_FILTER_DEPTH = 10;

Expand Down Expand Up @@ -90,14 +84,13 @@ export function filterProcessList(rootPid: number, processList: IProcessInfo[],
return children.reduce((prev, current) => prev.concat(current), [rootProcess]);
}

function getRawProcessList<T>(
rootPid: number,
queue: RequestQueue<T>,
callback: (result: T) => void,
transform: (pid: number, processList: IProcessInfo[]) => T,
function getRawProcessList(
callback: RequestCallback,
flags?: ProcessDataFlag
): void {
queue.push({ rootPid, callback });
const queue = globalRequestQueue;

queue.push(callback);

// Only make a new request if there is not currently a request in progress.
// This prevents too many requests from being made, there is also a crash that
Expand All @@ -106,10 +99,17 @@ function getRawProcessList<T>(
if (!requestInProgress) {
requestInProgress = true;
native.getProcessList((processList: IProcessInfo[]) => {
queue.forEach(r => {
r.callback(transform(r.rootPid, processList));
});
queue.length = 0;
// It is possible and valid for one callback to cause another to be added to the queue.
// To avoid orphaning those callbacks, we repeat the draining until the queue is empty.
// We use "queue.splice(0)" to atomically clear the queue, returning the batch to process.
// If any of those also made requests, we repeat until the callback chain completes.
//
// An alternative would be to splice the queue once and immediately reset requestInProgress
// before invoking callbacks: `CreateToolhelp32Snapshot` has safely completed at this point.
// However, that would circumvent the "too many requests" rate-limiting (?) concern above.
while (queue.length) {
queue.splice(0).forEach(cb => cb(processList));
}
requestInProgress = false;
}, flags || 0);
}
Expand All @@ -122,7 +122,7 @@ function getRawProcessList<T>(
* @param flags The flags for what process data should be included
*/
export function getProcessList(rootPid: number, callback: (processList: IProcessInfo[] | undefined) => void, flags?: ProcessDataFlag): void {
getRawProcessList(rootPid, processListRequestQueue, callback, filterProcessList, flags);
getRawProcessList(procs => callback(filterProcessList(rootPid, procs)), flags);
}

/**
Expand All @@ -141,5 +141,5 @@ export function getProcessCpuUsage(processList: IProcessInfo[], callback: (tree:
* @param flags Flags indicating what process data should be written on each node
*/
export function getProcessTree(rootPid: number, callback: (tree: IProcessTreeNode | undefined) => void, flags?: ProcessDataFlag): void {
getRawProcessList(rootPid, processTreeRequestQueue, callback, buildProcessTree, flags);
getRawProcessList(procs => callback(buildProcessTree(rootPid, procs)), flags);
}