Skip to content

Commit

Permalink
Merge pull request #589 from microsoft/robo/fix_ctrl_terminal
Browse files Browse the repository at this point in the history 
fix: controlling terminal on macOS
  • Loading branch information
@Tyriar
Tyriar authored Mar 31, 2023
2 parents 2978b1e + d22dfea commit 7217b42
Show file tree
Hide file tree
Showing 7 changed files with 94 additions and 141 deletions.
14 changes: 14 additions & 0 deletions binding.gyp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,20 @@
]
}
]
}],
['OS=="mac"', {
'targets': [
{
'target_name': 'spawn-helper',
'type': 'executable',
'sources': [
'src/unix/spawn-helper.cc',
],
"xcode_settings": {
"MACOSX_DEPLOYMENT_TARGET":"10.7"
}
},
]
}]
]
}
1 change: 1 addition & 0 deletions scripts/post-install.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ var BUILD_FILES = [
path.join(RELEASE_DIR, 'conpty_console_list.pdb'),
path.join(RELEASE_DIR, 'pty.node'),
path.join(RELEASE_DIR, 'pty.pdb'),
path.join(RELEASE_DIR, 'spawn-helper'),
path.join(RELEASE_DIR, 'winpty-agent.exe'),
path.join(RELEASE_DIR, 'winpty-agent.pdb'),
path.join(RELEASE_DIR, 'winpty.dll'),
Expand Down
2 changes: 1 addition & 1 deletion src/native.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ interface IWinptyNative {
}

interface IUnixNative {
fork(file: string, args: string[], parsedEnv: string[], cwd: string, cols: number, rows: number, uid: number, gid: number, useUtf8: boolean, onExitCallback: (code: number, signal: number) => void): IUnixProcess;
fork(file: string, args: string[], parsedEnv: string[], cwd: string, cols: number, rows: number, uid: number, gid: number, useUtf8: boolean, helperPath: string, onExitCallback: (code: number, signal: number) => void): IUnixProcess;
open(cols: number, rows: number): IUnixOpenProcess;
process(fd: number, pty: string): string;
process(pid: number): string;
Expand Down
139 changes: 32 additions & 107 deletions src/unix/pty.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,6 @@ pty_after_close(uv_handle_t *);
#if defined(__APPLE__) || defined(__OpenBSD__)
static void
pty_posix_spawn(char** argv, char** env,
char* cwd,
const struct termios *termp,
const struct winsize *winp,
int* master,
Expand All @@ -156,7 +155,7 @@ pty_posix_spawn(char** argv, char** env,
NAN_METHOD(PtyFork) {
Nan::HandleScope scope;

if (info.Length() != 10 ||
if (info.Length() != 11 ||
!info[0]->IsString() ||
!info[1]->IsArray() ||
!info[2]->IsArray() ||
Expand All @@ -166,9 +165,10 @@ NAN_METHOD(PtyFork) {
!info[6]->IsNumber() ||
!info[7]->IsNumber() ||
!info[8]->IsBoolean() ||
!info[9]->IsFunction()) {
!info[9]->IsString() ||
!info[10]->IsFunction()) {
return Nan::ThrowError(
"Usage: pty.fork(file, args, env, cwd, cols, rows, uid, gid, utf8, onexit)");
"Usage: pty.fork(file, args, env, cwd, cols, rows, uid, gid, utf8, helperPath, onexit)");
}

// file
Expand All @@ -189,7 +189,6 @@ NAN_METHOD(PtyFork) {

// cwd
Nan::Utf8String cwd_(info[3]);
char* cwd = strdup(*cwd_);

// size
struct winsize winp;
Expand Down Expand Up @@ -242,21 +241,26 @@ NAN_METHOD(PtyFork) {
cfsetispeed(term, B38400);
cfsetospeed(term, B38400);

// helperPath
Nan::Utf8String helper_path(info[9]);

pid_t pid;
int master;
#if defined(__APPLE__)
int argc = argv_->Length();
int argl = argc + 2;
int argl = argc + 4;
char **argv = new char*[argl];
argv[0] = strdup(*file);
argv[0] = strdup(*helper_path);
argv[1] = strdup(*cwd_);
argv[2] = strdup(*file);
argv[argl - 1] = NULL;
for (int i = 0; i < argc; i++) {
Nan::Utf8String arg(Nan::Get(argv_, i).ToLocalChecked());
argv[i + 1] = strdup(*arg);
argv[i + 3] = strdup(*arg);
}

pid_t pid;
int master;
#if defined(__APPLE__)
int err = 0;
pty_posix_spawn(argv, env, cwd, term, &winp, &master, &pid, &err);
int err = -1;
pty_posix_spawn(argv, env, term, &winp, &master, &pid, &err);
if (err != 0) {
Nan::ThrowError("posix_spawnp failed.");
goto done;
Expand All @@ -266,6 +270,17 @@ NAN_METHOD(PtyFork) {
goto done;
}
#else
int argc = argv_->Length();
int argl = argc + 2;
char **argv = new char*[argl];
argv[0] = strdup(*file);
argv[argl - 1] = NULL;
for (int i = 0; i < argc; i++) {
Nan::Utf8String arg(Nan::Get(argv_, i).ToLocalChecked());
argv[i + 1] = strdup(*arg);
}

char* cwd = strdup(*cwd_);
sigset_t newmask, oldmask;
struct sigaction sig_action;
// temporarily block all signals
Expand Down Expand Up @@ -352,7 +367,7 @@ NAN_METHOD(PtyFork) {
pty_baton *baton = new pty_baton();
baton->exit_code = 0;
baton->signal_code = 0;
baton->cb.Reset(v8::Local<v8::Function>::Cast(info[9]));
baton->cb.Reset(v8::Local<v8::Function>::Cast(info[10]));
baton->pid = pid;
baton->async.data = baton;

Expand All @@ -370,8 +385,6 @@ NAN_METHOD(PtyFork) {

for (int i = 0; i < envc; i++) free(env[i]);
delete[] env;

free(cwd);
#endif
return info.GetReturnValue().SetUndefined();
}
Expand Down Expand Up @@ -698,22 +711,13 @@ pty_getproc(int fd, char *tty) {
#if defined(__APPLE__)
static void
pty_posix_spawn(char** argv, char** env,
char* cwd,
const struct termios *termp,
const struct winsize *winp,
int* master,
pid_t* pid,
int* err) {
int low_fds[3];
size_t count = 0;
const char *p;
const char *z;
size_t l;
size_t k;
int seen_eacces;
const char *path;
char** env_iterator;
const char path_var[] = "PATH=";

for (; count < 3; count++) {
low_fds[count] = posix_openpt(O_RDWR);
Expand Down Expand Up @@ -769,16 +773,6 @@ pty_posix_spawn(char** argv, char** env,
posix_spawn_file_actions_adddup2(&acts, slave, STDERR_FILENO);
posix_spawn_file_actions_addclose(&acts, slave);
posix_spawn_file_actions_addclose(&acts, *master);
if (strlen(cwd)) {
if (__builtin_available(macOS 10.15, *)) {
posix_spawn_file_actions_addchdir_np(&acts, cwd);
} else {
*err = pthread_chdir_np(cwd);
if (*err != 0) {
goto done;
}
}
}

posix_spawnattr_t attrs;
posix_spawnattr_init(&attrs);
Expand All @@ -802,78 +796,9 @@ pty_posix_spawn(char** argv, char** env,
goto done;
}

// path resolution is copied from
// https://github.com/libuv/libuv/blob/7b84d5b0ecb737b4cc30ce63eade690d994e00a6/src/unix/process.c#L695-L764
if (strchr(argv[0], '/') != NULL) {
do
*err = posix_spawn(pid, argv[0], &acts, &attrs, argv, env);
while (*err == EINTR);
} else {

for (env_iterator = env; *env_iterator != NULL; env_iterator++) {
if (strncmp(*env_iterator, path_var, sizeof(path_var) - 1) == 0) {
/* Found "PATH=" at the beginning of the string */
path = *env_iterator + sizeof(path_var) - 1;
}
}

if (path == NULL)
path = _PATH_DEFPATH;

k = strnlen(argv[0], NAME_MAX + 1);
if (k > NAME_MAX)
goto done;

l = strnlen(path, PATH_MAX - 1) + 1;

for (p = path;; p = z) {
/* Compose the new process file from the entry in the PATH
* environment variable and the actual file name */
char b[PATH_MAX + NAME_MAX];
z = strchr(p, ':');
if (!z)
z = p + strlen(p);
if ((size_t)(z - p) >= l) {
if (!*z++)
break;

continue;
}
memcpy(b, p, z - p);
b[z - p] = '/';
memcpy(b + (z - p) + (z > p), argv[0], k + 1);

do
*err = posix_spawn(pid, b, &acts, &attrs, argv, env);
while (*err == EINTR);

switch (*err) {
case EACCES:
seen_eacces = 1;
break; /* continue search */
case ENOENT:
case ENOTDIR:
break; /* continue search */
default:
goto done;
}

if (!*z++)
break;
}

if (seen_eacces)
goto done;
}

// Restore the thread's working directory if it was changed.
if (strlen(cwd)) {
if (__builtin_available(macOS 10.15, *)) {
// __builtin_available is special and cannot be negated.
} else {
pthread_fchdir_np(-1);
}
}
do
*err = posix_spawn(pid, argv[0], &acts, &attrs, argv, env);
while (*err == EINTR);
done:
posix_spawn_file_actions_destroy(&acts);
posix_spawnattr_destroy(&attrs);
Expand Down
23 changes: 23 additions & 0 deletions src/unix/spawn-helper.cc
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#include <errno.h>
#include <fcntl.h>
#include <string.h>
#include <unistd.h>

int main (int argc, char** argv) {
char *slave_path = ttyname(STDIN_FILENO);
// open implicit attaches a process to a terminal device if:
// - process has no controlling terminal yet
// - O_NOCTTY is not set
close(open(slave_path, O_RDWR));

char *cwd = argv[1];
char *file = argv[2];
argv = &argv[2];

if (strlen(cwd) && chdir(cwd) == -1) {
_exit(1);
}

execvp(file, argv);
return 1;
}
47 changes: 15 additions & 32 deletions src/unixTerminal.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,38 +255,7 @@ if (process.platform !== 'win32') {
});
});
describe('spawn', () => {
if (process.platform === 'linux') {
it('should handle exec() errors', (done) => {
const term = new UnixTerminal('/bin/bogus.exe', []);
term.on('exit', (code, signal) => {
assert.strictEqual(code, 1);
done();
});
});
it('should handle chdir() errors', (done) => {
const term = new UnixTerminal('/bin/echo', [], { cwd: '/nowhere' });
term.on('exit', (code, signal) => {
assert.strictEqual(code, 1);
done();
});
});
} else if (process.platform === 'darwin') {
it('should handle exec() errors', (done) => {
try {
new UnixTerminal('/bin/bogus.exe', []);
done(new Error('should have failed'));
} catch {
done();
}
});
it('should handle chdir() errors', (done) => {
try {
new UnixTerminal('/bin/echo', [], { cwd: '/nowhere' });
done(new Error('should have failed'));
} catch (e) {
done();
}
});
if (process.platform === 'darwin') {
it('should return the name of the process', (done) => {
const term = new UnixTerminal('/bin/echo');
assert.strictEqual(term.process, '/bin/echo');
Expand Down Expand Up @@ -331,6 +300,20 @@ if (process.platform !== 'win32') {
});
});
}
it('should handle exec() errors', (done) => {
const term = new UnixTerminal('/bin/bogus.exe', []);
term.on('exit', (code, signal) => {
assert.strictEqual(code, 1);
done();
});
});
it('should handle chdir() errors', (done) => {
const term = new UnixTerminal('/bin/echo', [], { cwd: '/nowhere' });
term.on('exit', (code, signal) => {
assert.strictEqual(code, 1);
done();
});
});
it('should not leak child process', (done) => {
const count = cp.execSync('ps -ax | grep node | wc -l');
const term = new UnixTerminal('node', [ '-e', `
Expand Down
9 changes: 8 additions & 1 deletion src/unixTerminal.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,25 @@ import { ArgvOrCommandLine } from './types';
import { assign } from './utils';

let pty: IUnixNative;
let helperPath: string;
try {
pty = require('../build/Release/pty.node');
helperPath = '../build/Release/spawn-helper';
} catch (outerError) {
try {
pty = require('../build/Debug/pty.node');
helperPath = '../build/Debug/spawn-helper';
} catch (innerError) {
console.error('innerError', innerError);
// Re-throw the exception from the Release require if the Debug require fails as well
throw outerError;
}
}

helperPath = path.resolve(__dirname, helperPath);
helperPath = helperPath.replace('app.asar', 'app.asar.unpacked');
helperPath = helperPath.replace('node_modules.asar', 'node_modules.asar.unpacked');

const DEFAULT_FILE = 'sh';
const DEFAULT_NAME = 'xterm';
const DESTROY_SOCKET_TIMEOUT_MS = 200;
Expand Down Expand Up @@ -104,7 +111,7 @@ export class UnixTerminal extends Terminal {
};

// fork
const term = pty.fork(file, args, parsedEnv, cwd, this._cols, this._rows, uid, gid, (encoding === 'utf8'), onexit);
const term = pty.fork(file, args, parsedEnv, cwd, this._cols, this._rows, uid, gid, (encoding === 'utf8'), helperPath, onexit);

this._socket = new PipeSocket(term.fd);
if (encoding !== null) {
Expand Down

0 comments on commit 7217b42

Please sign in to comment.