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

uv_spawn is slow when parent process allocates mmap with MAP_JIT flag #3050

Closed
deepak1556 opened this issue Nov 21, 2020 · 28 comments · Fixed by electron/electron#27026 or sthagen/libuv-libuv#71
Labels
macos not-stale Issues that should never be marked stale

Comments

@deepak1556
Copy link

deepak1556 commented Nov 21, 2020

  • Version: tip-of-tree (1.x)
  • Platform: macOS Big Sur (11.0.1)

Repro Steps : https://github.com/deepak1556/libuv-spawn

The issue is derived from electron/electron#26143

Applications running on hardened runtime based on chromium/electron create mmap regions using MAP_JIT flag. With Big Sur the fork() calls done by uv_spawn have become slow. Theory is that before big sur fork() didn't have access to the JIT memory regions but they do now.

// Using the repro linked above

$ sudo dtruss -e ./out/Default/uv_spawn --use-jit

45600 fork()		 = 7815 0
42260 fork()		 = 7816 0

$ sudo dtruss -e ./out/Default/uv_spawn

558 fork()		 = 7801 0
391 fork()		 = 7800 0

Chromium's own child process spawning code doesn't suffer from this new behaviour because it uses posix_spawn.

Is it in the interest of libuv project to make the change to use posix_spawn atleast for macOS >= 11.0 ?

@bnoordhuis
Copy link
Member

Thanks for the repro. I don't run Big Sur so I have no way to test myself but your analysis sounds right.

posix_spawn() on macOS is an actual system call (on other Unices libc often emulates it through fork() + execve()) with a shortcut to spawn the new process directly. No memory mappings are copied, ergo, it's very fast.

Switching to posix_spawn() isn't an option in general - it's too limited for libuv's use cases - but it could be an option for macOS because you can use platform-specific extensions.

At the very least you'll need:

  • posix_spawn_file_actions_addchdir_np() to implement the cwd option to uv_spawn().
  • posix_spawnattr_set_uid_np() for uid
  • posix_spawnattr_set_gid_np() for gid
  • probably posix_spawnattr_set_groups_np() to emulate the setgroups() logic in src/unix/spawn.c

I won't be working on that myself but you or anyone else is welcome to send a pull request.

@marcello3d
Copy link

marcello3d commented Nov 23, 2020

The slow speed is unfortunate, but what's worse is that it locks the JavaScript thread (in Electron, freezing the app UI), before nodejs' spawn() call even returns.

I'm not familiar with what part of that is node vs libuv, but is it possible for this slow part to happen asynchronously (since the rest of the spawn api is async)?

@deepak1556
Copy link
Author

Thanks for the added context! I will work on the fix this week.

@pdesantis
Copy link

@deepak1556 Do you think this will be fixed this year? Is there any way we can help? Our Electron app is unusable on Big Sur because it relies heavily on child processes, so we’re trying to determine if we should build workarounds or contribute to the fix. Thanks again for the help!

@deepak1556
Copy link
Author

@pdesantis the issue is on our projects' December iteration task but can't promise a date and I haven't started on it. I also was not aware if anyone else wanted to work on it. So, feel free to contribute!

All the relevant api swaps that @bnoordhuis pointed are to be made in src/unix/process.c, specifically under fn uv__process_child_init and uv_spawn

@pdesantis
Copy link

@deepak1556 thanks! Taking a crack at it today, although don't get your hopes up too high since we're new to this codebase 😅. Will report back by the end of the day.

@pdesantis
Copy link

@deepak1556 @bnoordhuis we've made some progress here: descriptinc@58cf78e (shoutout to @jpcanepa!)

It's incomplete but seems promising so far. Using the repro project https://github.com/deepak1556/libuv-spawn we're seeing ls launch & exit times of 0.5ms & 3ms, which is inline with the times from the unsigned executable using fork (actually, they're slightly faster!)

Any tips on porting the file descriptor (options->stdio) logic?

@pdesantis
Copy link

Disregard my previous question, @jpcanepa posted a PR here #3064

@jpcanepa
Copy link
Contributor

jpcanepa commented Dec 8, 2020

I want to make sure that eveyrone involved here is aware that that PR (#3064) is in draft because I need help. I can continue trying to bang things into compiling correctly, but I kind of hope that someone (@deepak1556?, @bnoordhuis?) can chip in as well.

@bnoordhuis
Copy link
Member

As a bit of expectation management: I have very little time at the moment.

@jpcanepa
Copy link
Contributor

I've updated the PR so that now it builds correctly with CMake and Autotools, and there are only two open questions: a failing unit test case due to a behavior difference from the fork/execvp flow and a question about one of the posix_spawn_*_np function calls.

pdesantis added a commit to descriptinc/electron that referenced this issue Dec 14, 2020
Spawning child processes in an Electron application with a hardened runtime has become slow in macOS Big Sur.

This patch is a squashed version of libuv/libuv#3064, with the addition of API availability annotations to fix a build warning (since Electron compiles with the `-Wunguarded-availability-new` flag). This patch should be removed when libuv PR 3064 is merged.

Fixes: libuv/libuv#3050
Fixes: electron#26143
PR-URL: libuv/libuv#3064

Authored-by: Juan Pablo Canepa <jpcanepa@gmail.com>
Co-authored-by: Marcello Bastéa-Forte <marcello@descript.com>
Electron patch prepared by: Pat DeSantis <pdesantis3@gmail.com>
pdesantis added a commit to descriptinc/electron that referenced this issue Dec 14, 2020
Spawning child processes in an Electron application with a hardened runtime has become slow in macOS Big Sur.

This patch is a squashed version of libuv/libuv#3064, with the addition of API availability annotations to fix a build warning (since Electron compiles with the `-Wunguarded-availability-new` flag). This patch should be removed when libuv PR 3064 is merged.

Fixes: libuv/libuv#3050
Fixes: electron#26143
PR-URL: libuv/libuv#3064

Authored-by: Juan Pablo Canepa <jpcanepa@gmail.com>
Co-authored-by: Marcello Bastéa-Forte <marcello@descript.com>
Electron patch prepared by: Pat DeSantis <pdesantis3@gmail.com>
pdesantis added a commit to descriptinc/electron that referenced this issue Dec 15, 2020
Spawning child processes in an Electron application with a hardened runtime has become slow in macOS Big Sur.

This patch is a squashed version of libuv/libuv#3064, with the addition of API availability annotations to fix a build warning (since Electron compiles with the `-Wunguarded-availability-new` flag). This patch should be removed when libuv PR 3064 is merged.

Fixes: libuv/libuv#3050
Fixes: electron#26143
PR-URL: libuv/libuv#3064

Authored-by: Juan Pablo Canepa <jpcanepa@gmail.com>
Co-authored-by: Marcello Bastéa-Forte <marcello@descript.com>
Electron patch prepared by: Pat DeSantis <pdesantis3@gmail.com>
pdesantis added a commit to descriptinc/electron that referenced this issue Dec 15, 2020
Spawning child processes in an Electron application with a hardened runtime has become slow in macOS Big Sur.

This patch is a squashed version of libuv/libuv#3064, with the addition of API availability annotations to fix a build warning (since Electron compiles with the `-Wunguarded-availability-new` flag). This patch should be removed when libuv PR 3064 is merged.

Fixes: libuv/libuv#3050
Fixes: electron#26143
PR-URL: libuv/libuv#3064

Authored-by: Juan Pablo Canepa <jpcanepa@gmail.com>
Co-authored-by: Marcello Bastéa-Forte <marcello@descript.com>
Electron patch prepared by: Pat DeSantis <pdesantis3@gmail.com>
pdesantis added a commit to descriptinc/electron that referenced this issue Dec 15, 2020
Spawning child processes in an Electron application with a hardened runtime has become slow in macOS Big Sur.

This patch is a squashed version of libuv/libuv#3064, with the addition of API availability annotations to fix a build warning (since Electron compiles with the `-Wunguarded-availability-new` flag). This patch should be removed when libuv PR 3064 is merged.

Fixes: libuv/libuv#3050
Fixes: electron#26143
PR-URL: libuv/libuv#3064

Authored-by: Juan Pablo Canepa <jpcanepa@gmail.com>
Co-authored-by: Marcello Bastéa-Forte <marcello@descript.com>
Electron patch prepared by: Pat DeSantis <pdesantis3@gmail.com>
pdesantis added a commit to descriptinc/electron that referenced this issue Dec 22, 2020
Spawning child processes in an Electron application with a hardened runtime has become slow in macOS Big Sur.

This patch is a squashed version of libuv/libuv#3064, with the addition of API availability annotations to fix a build warning (since Electron compiles with the `-Wunguarded-availability-new` flag). This patch should be removed when libuv PR 3064 is merged.

Fixes: libuv/libuv#3050
Fixes: electron#26143
PR-URL: libuv/libuv#3064

Authored-by: Juan Pablo Canepa <jpcanepa@gmail.com>
Co-authored-by: Marcello Bastéa-Forte <marcello@descript.com>
Electron patch prepared by: Pat DeSantis <pdesantis3@gmail.com>
@stale stale bot added the stale label Dec 31, 2020
@libuv libuv deleted a comment from stale bot Jan 1, 2021
@stale stale bot removed the stale label Jan 1, 2021
@bnoordhuis bnoordhuis added the not-stale Issues that should never be marked stale label Jan 1, 2021
pdesantis added a commit to descriptinc/electron that referenced this issue Jan 7, 2021
Spawning child processes in an Electron application with a hardened runtime has become slow in macOS Big Sur.

This patch is a squashed version of libuv/libuv#3064, with the addition of API availability annotations to fix a build warning (since Electron compiles with the `-Wunguarded-availability-new` flag). This patch should be removed when libuv PR 3064 is merged.

Fixes: libuv/libuv#3050
Fixes: electron#26143
PR-URL: libuv/libuv#3064

Authored-by: Juan Pablo Canepa <jpcanepa@gmail.com>
Co-authored-by: Marcello Bastéa-Forte <marcello@descript.com>
Electron patch prepared by: Pat DeSantis <pdesantis3@gmail.com>
pdesantis added a commit to descriptinc/electron that referenced this issue Jan 8, 2021
Spawning child processes in an Electron application with a hardened runtime has become slow in macOS Big Sur.

This patch is a squashed version of libuv/libuv#3064, with the addition of API availability annotations to fix a build warning (since Electron compiles with the `-Wunguarded-availability-new` flag). This patch should be removed when libuv PR 3064 is merged.

Fixes: libuv/libuv#3050
Fixes: electron#26143
PR-URL: libuv/libuv#3064

Authored-by: Juan Pablo Canepa <jpcanepa@gmail.com>
Co-authored-by: Marcello Bastéa-Forte <marcello@descript.com>
Electron patch prepared by: Pat DeSantis <pdesantis3@gmail.com>
pdesantis added a commit to descriptinc/electron that referenced this issue Jan 8, 2021
Spawning child processes in an Electron application with a hardened runtime has become slow in macOS Big Sur.

This patch is a squashed version of libuv/libuv#3064, with the addition of API availability annotations to fix a build warning (since Electron compiles with the `-Wunguarded-availability-new` flag). This patch should be removed when libuv PR 3064 is merged.

Fixes: libuv/libuv#3050
Fixes: electron#26143
PR-URL: libuv/libuv#3064

Authored-by: Juan Pablo Canepa <jpcanepa@gmail.com>
Co-authored-by: Marcello Bastéa-Forte <marcello@descript.com>
Electron patch prepared by: Pat DeSantis <pdesantis3@gmail.com>
pdesantis added a commit to descriptinc/electron that referenced this issue Jan 11, 2021
Spawning child processes in an Electron application with a hardened runtime has become slow in macOS Big Sur.

This patch is a squashed version of libuv/libuv#3064, with the addition of API availability annotations to fix a build warning (since Electron compiles with the `-Wunguarded-availability-new` flag). This patch should be removed when libuv PR 3064 is merged.

Fixes: libuv/libuv#3050
Fixes: electron#26143
PR-URL: libuv/libuv#3064

Authored-by: Juan Pablo Canepa <jpcanepa@gmail.com>
Co-authored-by: Marcello Bastéa-Forte <marcello@descript.com>
Electron patch prepared by: Pat DeSantis <pdesantis3@gmail.com>
pdesantis added a commit to descriptinc/electron that referenced this issue Jun 10, 2021
Spawning child processes in an Electron application with a hardened runtime has become slow in macOS Big Sur.

This patch is a cherry-pick of libuv/libuv#3064. This patch should be removed when Electron's libuv version is updated to a version containing this fix.

Fixes: libuv/libuv#3050
Fixes: electron#26143
PR-URL: libuv/libuv#3064

Authored-by: Juan Pablo Canepa <jpcanepa@gmail.com>
Co-authored-by: Marcello Bastéa-Forte <marcello@descript.com>
pdesantis added a commit to descriptinc/electron that referenced this issue Jul 8, 2021
Spawning child processes in an Electron application with a hardened runtime has become slow in macOS Big Sur.

This patch is a cherry-pick of libuv/libuv#3064. This patch should be removed when Electron's libuv version is updated to a version containing this fix.

Fixes: libuv/libuv#3050
Fixes: electron#26143
PR-URL: libuv/libuv#3064

Authored-by: Juan Pablo Canepa <jpcanepa@gmail.com>
Co-authored-by: Marcello Bastéa-Forte <marcello@descript.com>
@jezmck
Copy link

jezmck commented Aug 3, 2021

What's the latest on this?

@fredleger
Copy link

fredleger commented Aug 3, 2021

I add to apply the fix for the latest build of gitkraken so i guess it's still ongoing or gitkraken has not included the fix released if it's exists.

What i don't get is that some apps suffered the same issue after Bug Sur was out (from memory i can only name VScode but they was more than 2 apps) and they managed to get a fix out quite quickly. Is there something special about this lib or it's just on Gitkraken side ?

pdesantis added a commit to descriptinc/electron that referenced this issue Oct 13, 2021
Spawning child processes in an Electron application with a hardened runtime has become slow in macOS Big Sur.

This patch is a cherry-pick of libuv/libuv#3064. This patch should be removed when Electron's libuv version is updated to a version containing this fix.

Fixes: libuv/libuv#3050
Fixes: electron#26143
PR-URL: libuv/libuv#3064

Authored-by: Juan Pablo Canepa <jpcanepa@gmail.com>
Co-authored-by: Marcello Bastéa-Forte <marcello@descript.com>
@herberthobregon
Copy link

Will this ever be solved? Any alternative to this program?

@tangix
Copy link

tangix commented Oct 28, 2021

With Big Sur possibly now in maintenance mode - is Kraken working better in Monterey?
Removing code-signing isn't a way forward in my book.

@idunno101
Copy link

If anyone wants a quick overview on this whole situation; this issue summarises it nicely.

Right now, the fix is at #3257 but Apple needs to diagnose a kernel bug before that can be merged.

pdesantis added a commit to descriptinc/electron that referenced this issue Nov 9, 2021
Spawning child processes in an Electron application with a hardened runtime has become slow in macOS Big Sur.

This patch is a cherry-pick of libuv/libuv#3064. This patch should be removed when Electron's libuv version is updated to a version containing this fix.

Fixes: libuv/libuv#3050
Fixes: electron#26143
PR-URL: libuv/libuv#3064

Authored-by: Juan Pablo Canepa <jpcanepa@gmail.com>
Co-authored-by: Marcello Bastéa-Forte <marcello@descript.com>
dreamerns pushed a commit to cryptagon/electron that referenced this issue Nov 11, 2021
Spawning child processes in an Electron application with a hardened runtime has become slow in macOS Big Sur.

This patch is a cherry-pick of libuv/libuv#3064. This patch should be removed when Electron's libuv version is updated to a version containing this fix.

Fixes: libuv/libuv#3050
Fixes: electron#26143
PR-URL: libuv/libuv#3064

Authored-by: Juan Pablo Canepa <jpcanepa@gmail.com>
Co-authored-by: Marcello Bastéa-Forte <marcello@descript.com>
@lingster
Copy link

I've just tried using GitKraken's new cli-terminal using Monterey and GitKraken grinds to a halt as the renderer consumes vast amounts of cpu time.
Running the workaround:
codesign --remove-signature /Applications/GitKraken.app/Contents/Frameworks/GitKraken\ Helper\ \(Renderer\).app
returns the following error:
/Applications/GitKraken.app/Contents/Frameworks/GitKraken Helper (Renderer).app: internal error in Code Signing subsystem

hoping that there will be a fix for this soon.

@luclu
Copy link

luclu commented Nov 22, 2021

@lingster try sudo ...

kevin-protopie pushed a commit to kevin-protopie/electron that referenced this issue Dec 16, 2021
Spawning child processes in an Electron application with a hardened runtime has become slow in macOS Big Sur.

This patch is a cherry-pick of libuv/libuv#3064. This patch should be removed when Electron's libuv version is updated to a version containing this fix.

Fixes: libuv/libuv#3050
Fixes: electron#26143
PR-URL: libuv/libuv#3064

Authored-by: Juan Pablo Canepa <jpcanepa@gmail.com>
Co-authored-by: Marcello Bastéa-Forte <marcello@descript.com>
kevin-protopie pushed a commit to kevin-protopie/electron that referenced this issue Dec 20, 2021
Spawning child processes in an Electron application with a hardened runtime has become slow in macOS Big Sur.

This patch is a cherry-pick of libuv/libuv#3064. This patch should be removed when Electron's libuv version is updated to a version containing this fix.

Fixes: libuv/libuv#3050
Fixes: electron#26143
PR-URL: libuv/libuv#3064

Authored-by: Juan Pablo Canepa <jpcanepa@gmail.com>
Co-authored-by: Marcello Bastéa-Forte <marcello@descript.com>
cvanwinkle added a commit to descriptinc/electron that referenced this issue Jan 15, 2022
Spawning child processes in an Electron application with a hardened runtime has become slow in macOS Big Sur.

This patch is a cherry-pick of libuv/libuv#3064. This patch should be removed when Electron's libuv version is updated to a version containing this fix.

Fixes: libuv/libuv#3050
Fixes: electron#26143
PR-URL: libuv/libuv#3064

Authored-by: Juan Pablo Canepa <jpcanepa@gmail.com>
Co-authored-by: Marcello Bastéa-Forte <marcello@descript.com>
cvanwinkle pushed a commit to descriptinc/electron that referenced this issue Jan 18, 2022
Spawning child processes in an Electron application with a hardened runtime has become slow in macOS Big Sur.

This patch is a cherry-pick of libuv/libuv#3064. This patch should be removed when Electron's libuv version is updated to a version containing this fix.

Fixes: libuv/libuv#3050
Fixes: electron#26143
PR-URL: libuv/libuv#3064

Authored-by: Juan Pablo Canepa <jpcanepa@gmail.com>
Co-authored-by: Marcello Bastéa-Forte <marcello@descript.com>
@r0kk
Copy link

r0kk commented Jan 22, 2022

I am using Big Sur and it everything worked perfectly well till previous week, and now it stopped. codesign ... doesn't seems to resolve the issue.

Is there any solution to this? Did this somehow reoccur? I like gitkraken, but will be force to stop paying yearly subscription, since it is not useable for me at the moment.

Thank you for any help.

cvanwinkle added a commit to descriptinc/electron that referenced this issue Feb 2, 2022
Spawning child processes in an Electron application with a hardened runtime has become slow in macOS Big Sur.

This patch is a cherry-pick of libuv/libuv#3064. This patch should be removed when Electron's libuv version is updated to a version containing this fix.

Fixes: libuv/libuv#3050
Fixes: electron#26143
PR-URL: libuv/libuv#3064

Authored-by: Juan Pablo Canepa <jpcanepa@gmail.com>
Co-authored-by: Marcello Bastéa-Forte <marcello@descript.com>
dreamerns pushed a commit to cryptagon/electron that referenced this issue Feb 4, 2022
Spawning child processes in an Electron application with a hardened runtime has become slow in macOS Big Sur.

This patch is a cherry-pick of libuv/libuv#3064. This patch should be removed when Electron's libuv version is updated to a version containing this fix.

Fixes: libuv/libuv#3050
Fixes: electron#26143
PR-URL: libuv/libuv#3064

Authored-by: Juan Pablo Canepa <jpcanepa@gmail.com>
Co-authored-by: Marcello Bastéa-Forte <marcello@descript.com>
@vtjnash vtjnash closed this as completed in 83efa3d Mar 2, 2022
cvanwinkle added a commit to descriptinc/electron that referenced this issue Mar 13, 2022
Spawning child processes in an Electron application with a hardened runtime has become slow in macOS Big Sur.

This patch is a cherry-pick of libuv/libuv#3064. This patch should be removed when Electron's libuv version is updated to a version containing this fix.

Fixes: libuv/libuv#3050
Fixes: electron#26143
PR-URL: libuv/libuv#3064

Authored-by: Juan Pablo Canepa <jpcanepa@gmail.com>
Co-authored-by: Marcello Bastéa-Forte <marcello@descript.com>
JeffroMF pushed a commit to JeffroMF/libuv that referenced this issue May 16, 2022
Applications running on hardened runtime based on Chromium/Electron
create mmap regions using MAP_JIT flag. With macOS Big Sur the fork()
calls done by uv_spawn have become slow. This is because fork() seems
to physically copy all JIT memory regions (no-copy-on-write). On
previous OS, these regions weren't accessible at all in the forked
process, explaining the regression.

The fix is to use posix_spawn() on macOS. This spawns a new process
directly, without copying any memory mappings.

Note that fork() is still used on earlier versions of macOS if the
necessary posix_spawn() platform-specific extensions are not available.

Fixes: libuv#3050
PR-URL: libuv#3064
Reviewed-By: Jameson Nash <vtjnash@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
JeffroMF pushed a commit to JeffroMF/libuv that referenced this issue May 16, 2022
Fixes: libuv#3050
Refs: libuv#3086
Refs: libuv#3064
Refs: libuv#3107
Refs: libuv#3064

This reverts commit 217fdf4, then fixes
several issues with it:

* remove error fast-cleanup code that triggers a nodejs bug

Refs: libuv#3107 (comment)

* protect posix_spawn from EINTR

This is not a documented valid error, but seems to have been observed.

* ignore setuid/setgid syscall

This kernel function is not permitted unless the process is setuid root,
so disable this syscall. Falling back to fork/exec should be okay for
the rare cases that the user decides they need to do setuid(getuid()) or
setuid(geteuid()) for the child.

Refs: libuv#3107 (comment)

* improve posix_spawn path search

Ports the improvements in musl back to this function

* fix some additional problems and formatting issues

We previously might fail to start a watcher, in rare failure cases,
resulting in a zombie that we would fail to kill. Also avoid creating
the signal-pipe unless required (addresses a review comment from Apple)

* fix fd->fd mapping reuse

There was a chance that when duplicating the fd's into stdio_count+fd we
might be closing a currently opened fd with that value.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
macos not-stale Issues that should never be marked stale
Projects
None yet