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

Correctly detect posix_spawn_file_actions_addchdir[_np] #326

Conversation

adamgundry
Copy link
Member

Fixes #303. The previous fix in #304 unfortunately didn't resolve the issue, but I've tested with strace that this patch results in the right syscall.

It appears that CPP macro names are case sensitive, and the code was previously checking for HAVE_posix_spawn_file_actions_addchdir whereas Autoconf defines HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR. This meant that calls to createProcess which set cwd to a Just value would always fail to use posix_spawn and instead fall back on fork, which is problematic when the parent process has a large residency.

Testing the fix revealed another issue, which is that _GNU_SOURCE must be defined before any glibc headers are included, otherwise the _np variant provided by glibc will not be declared.

…l#303)

It appears that CPP macro names are case sensitive, and the code was
previously checking for HAVE_posix_spawn_file_actions_addchdir
whereas Autoconf defines HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR. This
meant that calls to createProcess which set cwd to a Just value would
always fail to use posix_spawn and instead fall back on fork, which
is problematic when the parent process has a large residency.

Testing the fix revealed another issue, which is that _GNU_SOURCE must
be defined before any glibc headers are included, otherwise the _np
variant provided by glibc will not be declared.
@bgamari
Copy link
Contributor

bgamari commented Sep 23, 2024

Oof, autoconf strikes again.

@bgamari bgamari merged commit 91e7381 into haskell:master Sep 23, 2024
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dropping vfork support regressed performance when posix_spawn not available
2 participants