From 1437d3df15c1efae3164ae45c3285bd9959def5f Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Wed, 7 Aug 2024 02:00:50 -0700 Subject: [PATCH 1/3] darwin: workaround PROC_PIDLISTFDS on processes with no fds This has been causing various seemingly spurious CI failures as well as some failures on people running tests on beta builds. lix> ++(nix-collect-garbage-dry-run.sh:20) nix-store --gc --print-dead lix> ++(nix-collect-garbage-dry-run.sh:20) wc -l lix> finding garbage collector roots... lix> error: Listing pid 87261 file descriptors: Undefined error: 0 There is no real way to write a proper test for this, other than to start a process like the following: int main(void) { for (int i = 0; i < 1000; ++i) { close(i); } sleep(10000); } and then let Lix's gc look at it. I have a relatively high confidence this *will* fix the problem since I have manually confirmed the behaviour of the libproc call is as-unexpected, and it would perfectly explain the observed symptom. Fixes: https://git.lix.systems/lix-project/lix/issues/446 Change-Id: I67669b98377af17895644b3bafdf42fc33abd076 --- doc/manual/rl-next/haunted-gc-macos.md | 15 +++++++++++++++ src/libstore/platform/darwin.cc | 17 ++++++++++++++++- 2 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 doc/manual/rl-next/haunted-gc-macos.md diff --git a/doc/manual/rl-next/haunted-gc-macos.md b/doc/manual/rl-next/haunted-gc-macos.md new file mode 100644 index 000000000..3ce912b2d --- /dev/null +++ b/doc/manual/rl-next/haunted-gc-macos.md @@ -0,0 +1,15 @@ +--- +synopsis: "Fix unexpectedly-successful GC failures on macOS" +cls: 1723 +issues: fj#446 +credits: jade +category: Fixes +--- + +Has the following happened to you on macOS? This failure has been successfully eliminated, thanks to our successful deployment of advanced successful-failure detection technology (it's just `if (failed && errno == 0)`. Patent pendingnot really): + +``` +$ nix-store --gc --print-dead +finding garbage collector roots... +error: Listing pid 87261 file descriptors: Undefined error: 0 +``` diff --git a/src/libstore/platform/darwin.cc b/src/libstore/platform/darwin.cc index 1b591fde3..1f7e9be23 100644 --- a/src/libstore/platform/darwin.cc +++ b/src/libstore/platform/darwin.cc @@ -56,12 +56,27 @@ void DarwinLocalStore::findPlatformRoots(UncheckedRoots & unchecked) while (fdBufSize > fds.size() * sizeof(struct proc_fdinfo)) { // Reserve some extra size so we don't fail too much fds.resize((fdBufSize + fdBufSize / 8) / sizeof(struct proc_fdinfo)); + errno = 0; fdBufSize = proc_pidinfo( pid, PROC_PIDLISTFDS, 0, fds.data(), fds.size() * sizeof(struct proc_fdinfo) ); + // errno == 0???! Yes, seriously. This is because macOS has a + // broken syscall wrapper for proc_pidinfo that has no way of + // dealing with the system call successfully returning 0. It + // takes the -1 error result from the errno-setting syscall + // wrapper and turns it into a 0 result. But what if the system + // call actually returns 0? Then you get an errno of success. + // + // https://github.com/apple-opensource/xnu/blob/4f43d4276fc6a87f2461a3ab18287e4a2e5a1cc0/libsyscall/wrappers/libproc/libproc.c#L100-L110 + // https://git.lix.systems/lix-project/lix/issues/446#issuecomment-5483 + // FB14695751 if (fdBufSize <= 0) { - throw SysError("Listing pid %1% file descriptors", pid); + if (errno == 0) { + break; + } else { + throw SysError("Listing pid %1% file descriptors", pid); + } } } fds.resize(fdBufSize / sizeof(struct proc_fdinfo)); From f8fb335eb7b13fccd6fe8fca2280238a7a7170c8 Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Wed, 7 Aug 2024 02:28:47 -0700 Subject: [PATCH 2/3] build: declare all the deps as -isystem I don't know why but I was getting a spurious -Werror=switch-enum inside toml11. It does not make sense why it did not occur before, but it should be stopped. This was not done at an earlier stage to better match the legacy make build system, but we don't use it anyway. Change-Id: I636f8a71e8a0ba5e0feb80b435ae24c3af995c5d --- meson.build | 59 +++++++++++++++++++++++++++++++---------------------- 1 file changed, 35 insertions(+), 24 deletions(-) diff --git a/meson.build b/meson.build index 8577657d9..4e2f713ce 100644 --- a/meson.build +++ b/meson.build @@ -203,23 +203,23 @@ gc_opt = get_option('gc').disable_if( 'address' in get_option('b_sanitize'), error_message: 'gc does far too many memory crimes for ASan' ) -boehm = dependency('bdw-gc', required : gc_opt, version : '>=8.2.6') +boehm = dependency('bdw-gc', required : gc_opt, version : '>=8.2.6', include_type : 'system') configdata += { 'HAVE_BOEHMGC': boehm.found().to_int(), } -boost = dependency('boost', required : true, modules : ['container']) +boost = dependency('boost', required : true, modules : ['container'], include_type : 'system') # cpuid only makes sense on x86_64 cpuid_required = is_x64 ? get_option('cpuid') : false -cpuid = dependency('libcpuid', 'cpuid', required : cpuid_required) +cpuid = dependency('libcpuid', 'cpuid', required : cpuid_required, include_type : 'system') configdata += { 'HAVE_LIBCPUID': cpuid.found().to_int(), } # seccomp only makes sense on Linux seccomp_required = is_linux ? get_option('seccomp-sandboxing') : false -seccomp = dependency('libseccomp', 'seccomp', required : seccomp_required, version : '>=2.5.5') +seccomp = dependency('libseccomp', 'seccomp', required : seccomp_required, version : '>=2.5.5', include_type : 'system') if is_linux and not seccomp.found() warning('Sandbox security is reduced because libseccomp has not been found! Please provide libseccomp if it supports your CPU architecture.') endif @@ -227,19 +227,24 @@ configdata += { 'HAVE_SECCOMP': seccomp.found().to_int(), } -libarchive = dependency('libarchive', required : true) +libarchive = dependency('libarchive', required : true, include_type : 'system') brotli = [ - dependency('libbrotlicommon', required : true), - dependency('libbrotlidec', required : true), - dependency('libbrotlienc', required : true), + dependency('libbrotlicommon', required : true, include_type : 'system'), + dependency('libbrotlidec', required : true, include_type : 'system'), + dependency('libbrotlienc', required : true, include_type : 'system'), ] -openssl = dependency('libcrypto', 'openssl', required : true) +openssl = dependency('libcrypto', 'openssl', required : true, include_type : 'system') # FIXME: confirm we actually support such old versions of aws-sdk-cpp -aws_sdk = dependency('aws-cpp-sdk-core', required : false, version : '>=1.8') -aws_sdk_transfer = dependency('aws-cpp-sdk-transfer', required : aws_sdk.found(), fallback : ['aws_sdk', 'aws_cpp_sdk_transfer_dep']) +aws_sdk = dependency('aws-cpp-sdk-core', required : false, version : '>=1.8', include_type : 'system') +aws_sdk_transfer = dependency( + 'aws-cpp-sdk-transfer', + required : aws_sdk.found(), + fallback : ['aws_sdk', 'aws_cpp_sdk_transfer_dep'], + include_type : 'system', +) if aws_sdk.found() # The AWS pkg-config adds -std=c++11. # https://github.com/aws/aws-sdk-cpp/issues/2673 @@ -259,7 +264,12 @@ if aws_sdk.found() ) endif -aws_s3 = dependency('aws-cpp-sdk-s3', required : aws_sdk.found(), fallback : ['aws_sdk', 'aws_cpp_sdk_s3_dep']) +aws_s3 = dependency( + 'aws-cpp-sdk-s3', + required : aws_sdk.found(), + fallback : ['aws_sdk', 'aws_cpp_sdk_s3_dep'], + include_type : 'system', +) if aws_s3.found() # The AWS pkg-config adds -std=c++11. # https://github.com/aws/aws-sdk-cpp/issues/2673 @@ -276,30 +286,30 @@ configdata += { 'ENABLE_S3': aws_s3.found().to_int(), } -sqlite = dependency('sqlite3', 'sqlite', version : '>=3.6.19', required : true) +sqlite = dependency('sqlite3', 'sqlite', version : '>=3.6.19', required : true, include_type : 'system') -sodium = dependency('libsodium', 'sodium', required : true) +sodium = dependency('libsodium', 'sodium', required : true, include_type : 'system') -curl = dependency('libcurl', 'curl', required : true) +curl = dependency('libcurl', 'curl', required : true, include_type : 'system') -editline = dependency('libeditline', 'editline', version : '>=1.14', required : true) +editline = dependency('libeditline', 'editline', version : '>=1.14', required : true, include_type : 'system') -lowdown = dependency('lowdown', version : '>=0.9.0', required : true) +lowdown = dependency('lowdown', version : '>=0.9.0', required : true, include_type : 'system') # HACK(Qyriad): rapidcheck's pkg-config doesn't include the libs lol # Note: technically we 'check' for rapidcheck twice, for the internal-api-docs handling above, # but Meson will cache the result of the first one, and the required : arguments are different. -rapidcheck_meson = dependency('rapidcheck', required : enable_tests) +rapidcheck_meson = dependency('rapidcheck', required : enable_tests, include_type : 'system') rapidcheck = declare_dependency(dependencies : rapidcheck_meson, link_args : ['-lrapidcheck']) gtest = [ - dependency('gtest', required : enable_tests), - dependency('gtest_main', required : enable_tests), - dependency('gmock', required : enable_tests), - dependency('gmock_main', required : enable_tests), + dependency('gtest', required : enable_tests, include_type : 'system'), + dependency('gtest_main', required : enable_tests, include_type : 'system'), + dependency('gmock', required : enable_tests, include_type : 'system'), + dependency('gmock_main', required : enable_tests, include_type : 'system'), ] -toml11 = dependency('toml11', version : '>=3.7.0', required : true, method : 'cmake') +toml11 = dependency('toml11', version : '>=3.7.0', required : true, method : 'cmake', include_type : 'system') pegtl = dependency( 'pegtl', @@ -307,9 +317,10 @@ pegtl = dependency( required : true, method : 'cmake', modules : [ 'taocpp::pegtl' ], + include_type : 'system', ) -nlohmann_json = dependency('nlohmann_json', required : true) +nlohmann_json = dependency('nlohmann_json', required : true, include_type : 'system') # lix-doc is a Rust project provided via buildInputs and unfortunately doesn't have any way to be detected. # Just declare it manually to resolve this. From d1fd1dc8acbbbb3003fc633a9ffe7c8fb4b63e28 Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Wed, 7 Aug 2024 02:30:27 -0700 Subject: [PATCH 3/3] perl: un-autos your conf I definitely don't think we were using this, and it is probably an omission in the original autoconf deletion more than anything. Change-Id: Ib7c8082685e550575bca5af06f0e93adf982bd7c --- perl/configure.ac | 84 ----------------------------------------------- 1 file changed, 84 deletions(-) delete mode 100644 perl/configure.ac diff --git a/perl/configure.ac b/perl/configure.ac deleted file mode 100644 index 493f0c85c..000000000 --- a/perl/configure.ac +++ /dev/null @@ -1,84 +0,0 @@ -AC_INIT(nix-perl, m4_esyscmd([bash -c "echo -n $(cat ../.version)$VERSION_SUFFIX"])) -AC_CONFIG_SRCDIR(MANIFEST) -AC_CONFIG_AUX_DIR(../config) - -CFLAGS= -CXXFLAGS= -AC_PROG_CC -AC_PROG_CXX - -AC_CANONICAL_HOST - -# Use 64-bit file system calls so that we can support files > 2 GiB. -AC_SYS_LARGEFILE - -AC_DEFUN([NEED_PROG], -[ -AC_PATH_PROG($1, $2) -if test -z "$$1"; then - AC_MSG_ERROR([$2 is required]) -fi -]) - -NEED_PROG(perl, perl) -NEED_PROG(curl, curl) -NEED_PROG(bzip2, bzip2) -NEED_PROG(xz, xz) - -# Test that Perl has the open/fork feature (Perl 5.8.0 and beyond). -AC_MSG_CHECKING([whether Perl is recent enough]) -if ! $perl -e 'open(FOO, "-|", "true"); while () { print; }; close FOO or die;'; then - AC_MSG_RESULT(no) - AC_MSG_ERROR([Your Perl version is too old. Lix requires Perl 5.8.0 or newer.]) -fi -AC_MSG_RESULT(yes) - - -# Figure out where to install Perl modules. -AC_MSG_CHECKING([for the Perl installation prefix]) -perlversion=$($perl -e 'use Config; print $Config{version};') -perlarchname=$($perl -e 'use Config; print $Config{archname};') -AC_SUBST(perllibdir, [${libdir}/perl5/site_perl/$perlversion/$perlarchname]) -AC_MSG_RESULT($perllibdir) - -# Look for libsodium. -PKG_CHECK_MODULES([SODIUM], [libsodium], [CXXFLAGS="$SODIUM_CFLAGS $CXXFLAGS"]) - -# Check for the required Perl dependencies (DBI and DBD::SQLite). -perlFlags="-I$perllibdir" - -AC_ARG_WITH(dbi, AC_HELP_STRING([--with-dbi=PATH], - [prefix of the Perl DBI library]), - perlFlags="$perlFlags -I$withval") - -AC_ARG_WITH(dbd-sqlite, AC_HELP_STRING([--with-dbd-sqlite=PATH], - [prefix of the Perl DBD::SQLite library]), - perlFlags="$perlFlags -I$withval") - -AC_MSG_CHECKING([whether DBD::SQLite works]) -if ! $perl $perlFlags -e 'use DBI; use DBD::SQLite;' 2>&5; then - AC_MSG_RESULT(no) - AC_MSG_FAILURE([The Perl modules DBI and/or DBD::SQLite are missing.]) -fi -AC_MSG_RESULT(yes) - -AC_SUBST(perlFlags) - -PKG_CHECK_MODULES([NIX], [nix-store]) - -NEED_PROG([NIX], [nix]) - -# Expand all variables in config.status. -test "$prefix" = NONE && prefix=$ac_default_prefix -test "$exec_prefix" = NONE && exec_prefix='${prefix}' -for name in $ac_subst_vars; do - declare $name="$(eval echo "${!name}")" - declare $name="$(eval echo "${!name}")" - declare $name="$(eval echo "${!name}")" -done - -rm -f Makefile.config -ln -sfn ../mk mk - -AC_CONFIG_FILES([]) -AC_OUTPUT