microsoft / node-pty

Fork pseudoterminals in Node.JS
Other
1.47k stars 242 forks source link

Big Sur hardened runtime versus `fork` yields large slowdown in `node-pty.spawn()` #476

Closed starpit closed 2 years ago

starpit commented 3 years ago

Environment details

Issue description

When running node-pty.spawn() from within a macOS "hardened runtime" context, it appears that the fork() POSIX API is very slow [1] [2]. The referenced issues discuss the use of posix_spawn() versus fork(), and I see that node-pty's pty.cc uses fork().

This only seems to affect the combination of macOS 11 and the use of the macOS hardened runtime. You get this whenever signing node-pty.

We are seeing that each spawn call takes about 300ms, which is at least 100x slower than in other combinations.

[1] https://github.com/libuv/libuv/issues/3050 [2] https://github.com/electron/electron/issues/26143

jerch commented 3 years ago

I think we dont use any fork/exec specialties and should be able to transit to posix_spawn.

Things to watch out for though are:

Tyriar commented 3 years ago

We did hit this in VS Code and I think may have worked around it with a patch. It's an upstream issue in electron/libuv which should get fixed eventually so I'm not sure its worth the effort to try fix.

deepak1556 commented 3 years ago

@Tyriar the patch in vscode only handles the calls done to libuv for process spawn which currently is from child_process module in nodejs. Since node-pty manages its own process spawn implementation, the transition to posix_spawn family must also be done here separately.

1) posix_spawnattr_setsigdefault and posix_spawnattr_setsigmask can help achieve the changes in https://github.com/microsoft/node-pty/pull/218 2) posix_spawnattr_set_uid_np, posix_spawnattr_set_gid_np can help with it 3) > I suggest to do the transition only for Big Sur+, and leave other POSIX platforms untouched

Agree with this, the libuv patch does the same. I will take a stab at this in July if it hasn't been covered.

jerch commented 3 years ago

@deepak1556 Just a wild idea - does libuv-spawn expose enough primitives to use it instead of any own fork/spawn logic? I think all we need for a PTY process attach is the functionality of login_tty swapping the std channels, a setsid call and the TIOCSCTTY ioctl (resp. their macos counterparts). If thats not directly possible, it could be done by a small helper exec'ing into the final binary.

Not sure if there can be won anything, at least it sounds intriguing to me to delegate the low level process spawning to a well maintained lib node-pty depends on anyway.

Eugeny commented 3 years ago

Added a PoC implementation in #486