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

[DDW-462] Use "taskkill" to kill processes with pid on Windows #1162

Merged
Show file tree
Hide file tree
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
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ Changelog
- Implemented "New data layer migration" screen ([PR 1096](https://github.com/input-output-hk/daedalus/pull/1096))
- Implemented sending of `cert` and `key` with API requests in order to enable 2-way TLS authentication ([PR 1072](https://github.com/input-output-hk/daedalus/pull/1072))
- Implemented support for Cardano node "structured logging" ([PR 1092](https://github.com/input-output-hk/daedalus/pull/1092), [PR 1122](https://github.com/input-output-hk/daedalus/pull/1122))
- Implemented the IPC driven Cardano node / Daedalus communication ([PR 1075](https://github.com/input-output-hk/daedalus/pull/1075), [PR 1107](https://github.com/input-output-hk/daedalus/pull/1107), [PR 1109](https://github.com/input-output-hk/daedalus/pull/1109), [PR 1115](https://github.com/input-output-hk/daedalus/pull/1115), [PR 1118](https://github.com/input-output-hk/daedalus/pull/1118), [PR 1119](https://github.com/input-output-hk/daedalus/pull/1119))
- Implemented the IPC driven Cardano node / Daedalus communication ([PR 1075](https://github.com/input-output-hk/daedalus/pull/1075), [PR 1107](https://github.com/input-output-hk/daedalus/pull/1107), [PR 1109](https://github.com/input-output-hk/daedalus/pull/1109), [PR 1115](https://github.com/input-output-hk/daedalus/pull/1115), [PR 1118](https://github.com/input-output-hk/daedalus/pull/1118), [PR 1119](https://github.com/input-output-hk/daedalus/pull/1119), [PR 1162](https://github.com/input-output-hk/daedalus/pull/1162))
- Improved the loading UX ([PR 723](https://github.com/input-output-hk/daedalus/pull/723))
- Improved the NTP check handling ([PR 1086](https://github.com/input-output-hk/daedalus/pull/1086), [PR 1149](https://github.com/input-output-hk/daedalus/pull/1149), [PR 1158](https://github.com/input-output-hk/daedalus/pull/1158))
- Improved the transaction details text selection ([PR 1073](https://github.com/input-output-hk/daedalus/pull/1073), [PR 1095](https://github.com/input-output-hk/daedalus/pull/1095))
Expand Down
21 changes: 17 additions & 4 deletions source/main/cardano/CardanoNode.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// @flow
import Store from 'electron-store';
import type { ChildProcess, spawn } from 'child_process';
import type { ChildProcess, spawn, exec } from 'child_process';
import type { WriteStream } from 'fs';
import { toInteger } from 'lodash';
import environment from '../../common/environment';
Expand All @@ -16,6 +16,7 @@ type Logger = {

type Actions = {
spawn: spawn,
exec: exec,
readFileSync: (path: string) => Buffer,
createWriteStream: (path: string, options?: Object) => WriteStream,
broadcastTlsConfig: (config: ?TlsConfig) => void,
Expand Down Expand Up @@ -301,6 +302,7 @@ export class CardanoNode {
await this.start(_config, isForced);
} catch (error) {
_log.info(`CardanoNode#restart: Could not restart cardano-node "${error}"`);
this._changeToState(CardanoNodeStates.ERRORED);
return Promise.reject(error);
}
}
Expand Down Expand Up @@ -527,13 +529,24 @@ export class CardanoNode {
_killProcessWithName = async (pid: number, name: string): Promise<void> => {
const { _config } = this;
try {
process.kill(pid);
await promisedCondition(() => !this._isProcessRunning(pid, name), _config.killTimeout);
if (!environment.isWindows()) {
this._log.info('CardanoNode: using "process.kill(pid)" to kill.');
process.kill(pid);
} else {
// https://docs.microsoft.com/en-us/windows-server/administration/windows-commands/taskkill
const windowsKillCmd = `taskkill /pid ${pid} /t /f`;
this._log.info(`CardanoNode (Windows): using "${windowsKillCmd}" to kill.`);
this._actions.exec(windowsKillCmd);
}
await promisedCondition(async () => (
(await this._isProcessRunning(pid, name)) === false
), _config.killTimeout);

this._log.info(`CardanoNode: successfuly killed ${name} process (PID: ${pid})`);
return Promise.resolve();
} catch (error) {
this._log.info(
`CardanoNode: _killPreviousProcess returned an error attempting to kill ${name}
`CardanoNode: _killProcessWithName returned an error attempting to kill ${name}
process (PID: ${pid}). Error: ${JSON.stringify(error)}`
);
return Promise.reject(error);
Expand Down
3 changes: 2 additions & 1 deletion source/main/cardano/setup.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// @flow
import { createWriteStream, readFileSync } from 'fs';
import { spawn } from 'child_process';
import { spawn, exec } from 'child_process';
import { BrowserWindow } from 'electron';
import { Logger } from '../../common/logging';
import { prepareArgs } from './config';
Expand Down Expand Up @@ -55,6 +55,7 @@ export const setupCardano = (
const cardanoNode = new CardanoNode(Logger, {
// Dependencies on node.js apis are passed as props to ease testing
spawn,
exec,
readFileSync,
createWriteStream,
broadcastTlsConfig: (config: ?TlsConfig) => {
Expand Down