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

refactor(electron): ♻️ use async model for ChildProcessHelper #1455

Merged
merged 2 commits into from
Oct 28, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
118 changes: 64 additions & 54 deletions src/electron/go_vpn_tunnel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,15 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import {execFile} from 'child_process';
import {powerMonitor} from 'electron';
import {platform} from 'os';
import {promisify} from 'util';

import {pathToEmbeddedBinary} from '../infrastructure/electron/app_paths';
import {ShadowsocksSessionConfig} from '../www/app/tunnel';
import {TunnelStatus} from '../www/app/tunnel';
import * as errors from '../www/model/errors';
import {ErrorCode, fromErrorCode, UnexpectedPluginError} from '../www/model/errors';

import {ChildProcessHelper} from './process';
import {ChildProcessHelper, ProcessTerminatedExitCodeError, ProcessTerminatedSignalError} from './process';
import {RoutingDaemon} from './routing_service';
import {VpnTunnel} from './vpn_tunnel';

Expand Down Expand Up @@ -102,7 +100,9 @@ export class GoVpnTunnel implements VpnTunnel {
this.isUdpEnabled = await checkConnectivity(this.config);
}
console.log(`UDP support: ${this.isUdpEnabled}`);
await this.tun2socks.start(this.isUdpEnabled);

// Don't await here because we want to launch both binaries
this.tun2socks.start(this.isUdpEnabled);

await this.routing.start();
}
Expand Down Expand Up @@ -161,7 +161,7 @@ export class GoVpnTunnel implements VpnTunnel {

// Restart tun2socks.
await this.tun2socks.stop();
await this.tun2socks.start(this.isUdpEnabled);
this.tun2socks.start(this.isUdpEnabled);
}

// Use #onceDisconnected to be notified when the tunnel terminates.
Expand All @@ -178,7 +178,9 @@ export class GoVpnTunnel implements VpnTunnel {
try {
await this.tun2socks.stop();
} catch (e) {
console.error(`could not stop tun2socks: ${e.message}`);
if (!(e instanceof ProcessTerminatedSignalError)) {
console.error(`could not stop tun2socks: ${e.message}`);
}
}

try {
Expand Down Expand Up @@ -214,13 +216,14 @@ export class GoVpnTunnel implements VpnTunnel {
// outline-go-tun2socks is a Go program that processes IP traffic from a TUN/TAP device
// and relays it to a Shadowsocks proxy server.
class GoTun2socks {
private process: ChildProcessHelper;
private stopRequested = false;
private readonly process: ChildProcessHelper;

constructor(private config: ShadowsocksSessionConfig) {
constructor(private readonly config: ShadowsocksSessionConfig) {
this.process = new ChildProcessHelper(pathToEmbeddedBinary('outline-go-tun2socks', 'tun2socks'));
}

async start(isUdpEnabled: boolean) {
async start(isUdpEnabled: boolean): Promise<void> {
// ./tun2socks.exe \
// -tunName outline-tap0 -tunDNS 1.1.1.1,9.9.9.9 \
// -tunAddr 10.0.85.2 -tunGw 10.0.85.1 -tunMask 255.255.255.0 \
Expand All @@ -241,39 +244,56 @@ class GoTun2socks {
args.push('-dnsFallback');
}

return new Promise<void>((resolve, reject) => {
this.process.onExit = (code?: number) => {
reject(errors.fromErrorCode(code ?? errors.ErrorCode.UNEXPECTED));
};
this.stopRequested = false;
let autoRestart = false;
do {
if (autoRestart) {
console.warn(`tun2socks exited unexpectedly. Restarting...`);
}
autoRestart = false;
this.process.onStdErr = (data?: string | Buffer) => {
if (!data?.toString().includes('tun2socks running')) {
return;
if (data?.toString().includes('tun2socks running')) {
console.debug('tun2socks started');
autoRestart = true;
this.process.onStdErr = null;
}
console.debug('tun2socks started');
this.process.onExit = async (code?: number, signal?: string) => {
// The process exited unexpectedly, restart it.
console.warn(`tun2socks exited unexpectedly with signal: ${signal}, code: ${code}. Restarting...`);
await this.start(isUdpEnabled);
};
this.process.onStdErr = null;
resolve();
};
this.process.launch(args);
});
try {
await this.process.launch(args);
console.info('tun2socks exited with no errors');
} catch (e) {
console.error(`tun2socks terminated due to ${e}`);
}
} while (!this.stopRequested && autoRestart);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You removed the nested start calls, nice! I was worried about the stack growth.

}

async stop() {
return new Promise<void>(resolve => {
this.process.onExit = (code?: number, signal?: string) => {
console.log(`tun2socks stopped with signal: ${signal}, code: ${code}.`);
resolve();
};
this.process.stop();
});
stop() {
this.stopRequested = true;
return this.process.stop();
}

/**
* Checks connectivity and exits with an error code as defined in `errors.ErrorCode`.
* If exit code is not zero, a `ProcessTerminatedExitCodeError` might be thrown.
* -tun* and -dnsFallback options have no effect on this mode.
*/
checkConnectivity() {
console.debug('using tun2socks to check connectivity');
return this.process.launch([
'-proxyHost',
this.config.host || '',
'-proxyPort',
`${this.config.port}`,
'-proxyPassword',
this.config.password || '',
'-proxyCipher',
this.config.method || '',
'-checkConnectivity',
]);
}

enableDebugMode() {
this.process.enableDebugMode();
this.process.isDebugModeEnabled = true;
}
}

Expand All @@ -282,27 +302,17 @@ class GoTun2socks {
// forwarding and validates the proxy credentials. Resolves with a boolean indicating whether UDP
// forwarding is supported. Throws if the checks fail or if the process fails to start.
async function checkConnectivity(config: ShadowsocksSessionConfig) {
const args = [];
args.push('-proxyHost', config.host || '');
args.push('-proxyPort', `${config.port}`);
args.push('-proxyPassword', config.password || '');
args.push('-proxyCipher', config.method || '');
// Checks connectivity and exits with an error code as defined in `errors.ErrorCode`
// -tun* and -dnsFallback options have no effect on this mode.
args.push('-checkConnectivity');

const exec = promisify(execFile);
try {
await exec(pathToEmbeddedBinary('outline-go-tun2socks', 'tun2socks'), args);
await new GoTun2socks(config).checkConnectivity();
return true;
} catch (e) {
console.error(`connectivity check failed: ${e}`);
const code = e.status;
if (code === errors.ErrorCode.UDP_RELAY_NOT_ENABLED) {
// Don't treat lack of UDP support as an error, relay to the caller.
return false;
console.error(`connectivity check error: ${e}`);
if (e instanceof ProcessTerminatedExitCodeError) {
if (e.exitCode === ErrorCode.UDP_RELAY_NOT_ENABLED) {
return false;
}
throw fromErrorCode(e.exitCode);
}
// Treat the absence of a code as an unexpected error.
throw errors.fromErrorCode(code ?? errors.ErrorCode.UNEXPECTED);
throw new UnexpectedPluginError();
}
return true;
}
160 changes: 95 additions & 65 deletions src/electron/process.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,27 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import {ChildProcess, spawn} from 'child_process';
import * as path from 'path';
import {ChildProcess, spawn} from 'node:child_process';
import {basename} from 'node:path';
import process from 'node:process';

/**
* A child process is terminated abnormally, caused by a non-zero exit code.
*/
export class ProcessTerminatedExitCodeError extends Error {
constructor(public readonly exitCode: number) {
super(`Process terminated by non-zero exit code: ${exitCode}`);
}
}

/**
* A child process is terminated abnormally, caused by a signal string.
*/
export class ProcessTerminatedSignalError extends Error {
constructor(public readonly signal: string) {
super(`Process terminated by signal: ${signal}`);
}
}

// Simple "one shot" child process launcher.
//
Expand All @@ -22,90 +41,101 @@ import * as path from 'path';
// (which may be immediately after calling #startInternal if, e.g. the binary cannot be
// found).
export class ChildProcessHelper {
private process?: ChildProcess;
protected isInDebugMode = false;
private readonly processName: string;
private childProcess?: ChildProcess = null;
private waitProcessToExit?: Promise<void>;

/**
* Whether to enable verbose logging for the process. Must be called before launch().
*/
public isDebugModeEnabled = false;

private exitListener?: (code?: number, signal?: string) => void;
private stdErrListener?: (data?: string | Buffer) => void;

constructor(private path: string) {}
constructor(private readonly path: string) {
this.processName = basename(this.path);
}

/**
* Starts the process with the given args. If enableDebug() has been called, then the process is
* started in verbose mode if supported.
* Start the process with the given args and wait for the process to exit. If `isDebugModeEnabled`
* is `true`, the process is started in verbose mode if supported.
*
* If the process does not exist normally (i.e., exit code !== 0 or received a signal), it will
* throw either `ProcessTerminatedExitCodeError` or `ProcessTerminatedSignalError`.
*
* @param args The args for the process
*/
launch(args: string[]) {
this.process = spawn(this.path, args);
const processName = path.basename(this.path);

const onExit = (code?: number, signal?: string) => {
if (this.process) {
this.process.removeAllListeners();
}
if (this.exitListener) {
this.exitListener(code, signal);
}

logExit(processName, code, signal);
};
const onStdErr = (data?: string | Buffer) => {
if (this.isInDebugMode) {
console.error(`[STDERR - ${processName}]: ${data}`);
}
if (this.stdErrListener) {
this.stdErrListener(data);
}
};
this.process.stderr.on('data', onStdErr.bind(this));

if (this.isInDebugMode) {
// Redirect subprocess output while bypassing the Node console. This makes sure we don't
// send web traffic information to Sentry.
this.process.stdout.pipe(process.stdout);
this.process.stderr.pipe(process.stderr);
async launch(args: string[]): Promise<void> {
if (this.childProcess) {
throw new Error(`subprocess ${this.processName} has already been launched`);
}
this.childProcess = spawn(this.path, args);
return (this.waitProcessToExit = new Promise((resolve, reject) => {
const onExit = (code?: number, signal?: string) => {
if (this.childProcess) {
this.childProcess.removeAllListeners();
this.childProcess = null;
} else {
// When listening to both the 'exit' and 'error' events, guard against accidentally
// invoking handler functions multiple times.
return;
}

logExit(this.processName, code, signal);
if (code === 0) {
resolve();
} else if (code) {
reject(new ProcessTerminatedExitCodeError(code));
} else {
reject(new ProcessTerminatedSignalError(signal));
}
};

const onStdErr = (data?: string | Buffer) => {
if (this.isDebugModeEnabled) {
console.error(`[STDERR - ${this.processName}]: ${data}`);
}
if (this.stdErrListener) {
this.stdErrListener(data);
}
};
this.childProcess.stderr.on('data', onStdErr.bind(this));

if (this.isDebugModeEnabled) {
// Redirect subprocess output while bypassing the Node console. This makes sure we don't
// send web traffic information to Sentry.
this.childProcess.stdout.pipe(process.stdout);
this.childProcess.stderr.pipe(process.stderr);
}

// We have to listen for both events: error means the process could not be launched and in that
// case exit will not be invoked.
this.process.on('error', onExit.bind(this));
this.process.on('exit', onExit.bind(this));
// We have to listen for both events: error means the process could not be launched and in that
// case exit will not be invoked.
this.childProcess.on('error', onExit.bind(this));
this.childProcess.on('exit', onExit.bind(this));
}));
}

// Use #onExit to be notified when the process exits.
stop() {
if (!this.process) {
/**
* Try to kill the process and wait for the process to exit.
*
* If the process does not exist normally (i.e., exit code !== 0 or received a signal), it will
* throw either `ProcessTerminatedExitCodeError` or `ProcessTerminatedSignalError`.
*/
stop(): Promise<void> {
if (!this.childProcess) {
// Never started.
if (this.exitListener) {
this.exitListener(null, null);
}
return;
}

this.process.kill();
}

set onExit(newListener: ((code?: number, signal?: string) => void) | undefined) {
this.exitListener = newListener;
this.childProcess.kill();
return this.waitProcessToExit;
}

set onStdErr(listener: ((data?: string | Buffer) => void) | undefined) {
this.stdErrListener = listener;
if (!this.stdErrListener && !this.isDebugModeEnabled) {
this.process.stderr.removeAllListeners();
this.childProcess?.stderr.removeAllListeners();
}
}

/**
* Enables verbose logging for the process. Must be called before launch().
*/
enableDebugMode() {
this.isInDebugMode = true;
}

get isDebugModeEnabled() {
return this.isInDebugMode;
}
}

function logExit(processName: string, exitCode?: number, signal?: string) {
Expand Down