Skip to content

Commit

Permalink
Revert "fix: prevent deadlocks in node manager operations"
Browse files Browse the repository at this point in the history
This reverts commit ce92e41.
  • Loading branch information
devin-ai-integration[bot] committed Dec 19, 2024
1 parent 3a95cae commit 62df044
Showing 1 changed file with 33 additions and 71 deletions.
104 changes: 33 additions & 71 deletions apps/shinkai-visor-e2e/src/utils/node-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ export class NodeManager {

private node: ChildProcess | undefined;
private nodeExecPath: string;
private operationLock: Promise<void> = Promise.resolve();

constructor(nodeExecPath?: string) {
this.nodeExecPath =
Expand All @@ -36,17 +35,6 @@ export class NodeManager {
);
}

private async acquireLock(): Promise<() => void> {
let release: () => void;
const newLock = new Promise<void>((resolve) => {
release = resolve;
});
const oldLock = this.operationLock;
this.operationLock = newLock;
await oldLock;
return release!;
}

private async spawnNode(
command: string,
options: {
Expand All @@ -65,31 +53,19 @@ export class NodeManager {
stdio: 'pipe',
shell: true,
});

const cleanup = () => {
childProcess.removeAllListeners();
childProcess.stdout?.removeAllListeners();
childProcess.stderr?.removeAllListeners();
};

childProcess.on('close', (err) => {
logger(`close with code ${String(err)}`);
cleanup();
reject(err);
});

childProcess.on('error', (err) => {
logger(`error ${String(err)}`);
});

childProcess.stderr.on('error', (data) => {
logger(String(data));
});

childProcess.stdout.on('error', (data) => {
logger(String(data));
});

if (options.pipeLogs) {
childProcess.stderr.on('data', (data) => {
logger(data.toString());
Expand All @@ -98,21 +74,17 @@ export class NodeManager {
logger(data.toString());
});
}

if (options.readyMatcher) {
const timeoutRef = setTimeout(() => {
childProcess.kill();
cleanup();
reject(
`ready matcher timeout after ${options.readyMatcherTimeoutMs}`,
);
}, options.readyMatcherTimeoutMs ?? 15000);

childProcess.stdout?.on('data', (chunk: Buffer) => {
if (options.readyMatcher?.test(chunk.toString())) {
logger(`process ready, with readyMatcher:${chunk.toString()}`);
clearTimeout(timeoutRef);
cleanup();
resolve(childProcess);
}
});
Expand All @@ -123,55 +95,45 @@ export class NodeManager {
}

async startNode(pristine: boolean, nodeOptions?: object): Promise<void> {
const release = await this.acquireLock();
console.log('starting node');
try {
const mergedOptions = {
...this.defaultNodeOptions,
...(nodeOptions || {}),
};
if (pristine) {
this.resetToPristine(mergedOptions.NODE_STORAGE_PATH);
}
const nodeEnv = Object.entries(mergedOptions)
.map(([key, value]) => {
return `${key}="${value}"`;
})
.join(' ');

this.node = await this.spawnNode(`${nodeEnv} ${this.nodeExecPath}`, {
pipeLogs: true,
logsId: 'shinkai-node',
readyMatcher: /Server::run/,
});
console.log('node started');
} finally {
release();
const mergedOptions = {
...this.defaultNodeOptions,
...(nodeOptions || {}),
};
if (pristine) {
this.resetToPristine(mergedOptions.NODE_STORAGE_PATH);
}
const nodeEnv = Object.entries(mergedOptions)
.map(([key, value]) => {
return `${key}="${value}"`;
})
.join(' ');

this.node = await this.spawnNode(`${nodeEnv} ${this.nodeExecPath}`, {
pipeLogs: true,
logsId: 'shinkai-node',
readyMatcher: /Server::run/,
});
console.log('node started');
}

async stopNode(): Promise<void> {
const release = await this.acquireLock();
console.log('stopping node');
try {
if (!this.node) {
return Promise.resolve();
}
this.node.kill();
await new Promise<void>((resolve) => {
const timeout = setTimeout(() => {
console.warn('stopping node timeout');
resolve();
}, 5000);
this.node.once('exit', () => {
console.log('stopping node success');
clearTimeout(timeout);
resolve();
});
});
this.node = undefined;
} finally {
release();
if (!this.node) {
return Promise.resolve();
}
this.node.kill();
await new Promise<void>((resolve) => {
const timeout = setTimeout(() => {
console.warn('stopping node timeout');
resolve();
}, 5000);
this.node.once('exit', () => {
console.log('stopping node success');
clearTimeout(timeout);
resolve();
});
});
this.node = undefined;
}
}

0 comments on commit 62df044

Please sign in to comment.