Skip to content

Got segfault in devtools::test(filter = "compile-package") - #84

Closed
mns-nordicals wants to merge 1 commit into
t-kalinowski:mainfrom
mns-nordicals:package-segfaults
Closed

Got segfault in devtools::test(filter = "compile-package")#84
mns-nordicals wants to merge 1 commit into
t-kalinowski:mainfrom
mns-nordicals:package-segfaults

Conversation

@mns-nordicals

Copy link
Copy Markdown
Contributor

Below I provide an ai summary of the fix (codex did this mostly by it self)

Problem

After fetching the latest changes to quickr to my fork and running tests I get two failed tests in devtools::test(filter = "compile-package")

── Failed tests ────────────────────────────────────────────────────────────
Error (test-compile-package.R:112:3): compile_package runs pkgload::load_all and writes src outputs
Error: pkgload::load_all() failed with status 139:

[...]

Error (test-compile-package.R:155:3): pkgload::load_all writes outputs and resolves anonymous quick() names
Error in `run_r(code)`: R subprocess failed with status: 139

Solution

It turns out that the solution was that the tests were running with an older installed version of quickr and running devtools::install() fixed the problem. I was also surprised to it had passed github automated tests.

Nevertheless, before I figured out the problem I got codex to try an investigate the issue. After learning the "real" problem it still thinks it's changes makes the process more robust. So if you think the same you can merge these changes and if you think they are not necessary and we should use the installed version in the package tests then you are free to disregard this PR.

note, I have a local folder called .ai that i added to rbuldignore.

Segfault investigation: test-compile-package.R (AI summary of changes and fix)

Summary

The segfault was caused by the subprocess loading an older installed quickr instead of the dev version. The installed version generated a QuickrEntries array without a {NULL, NULL, 0} sentinel, which is required by R_registerRoutines(). When the test package’s DLL was loaded, R_registerRoutines() walked past the array and crashed (address 0x1). Ensuring the subprocess loads the dev quickr fixes the crash because the dev version correctly emits the sentinel.

After running devtools::install() on main, the segfault stopped. That indicates the crash was triggered because the subprocesses were loading an installed quickr that was older than the dev sources. The change still improves robustness because it removes the implicit dependency on whatever installed version happens to be on the library path.

Environment / Toolchain

From the failing run and checks:

  • OS: Fedora Linux 43 (Workstation Edition)
  • R: 4.5.2 (2025-10-31)
  • C compiler: GCC 15.2.1 (Red Hat 15.2.1-5 / 20251211)
  • Fortran compiler: GNU Fortran 15.2.1 (Red Hat 15.2.1-5 / 20251211)

Evidence / Reproduction

  • The segfault occurred during pkgload::load_all() in a subprocess while loading the temporary test package DLL.
  • With the installed quickr, the generated quickr_entrypoints.c had:
    static const R_ExternalMethodDef QuickrEntries[] = {
      {"add_ab_", (DL_FUNC) &add_ab_, -1}
    };
    (no terminating {NULL, NULL, 0} entry)
  • R_registerRoutines() expects a sentinel-terminated array. Without it, it reads past the array and crashes (segfault at address 0x1).

Fix Implemented

1) Ensure compile_package() subprocess loads dev quickr

When compile_package() launches a fresh R subprocess, it now preloads the dev quickr (if the current session is a dev package) before calling pkgload::load_all('.'):

  • File: R/compile-package.R (lines 31–53)
  • Logic:
    • Detect dev package via pkgload::is_dev_package("quickr")
    • Resolve dev path via getNamespaceInfo(asNamespace("quickr"), "path")
    • Build r_code like:
      pkgload::load_all(<dev-path>, quiet = TRUE); pkgload::load_all(quiet = TRUE)

2) Ensure test subprocesses do the same

The tests that use run_r() now preload the dev quickr before loading the test package:

  • File: tests/testthat/test-compile-package.R (lines 81, 151–157, 173–179)
  • Adds:
    quickr_dev_path <- getNamespaceInfo(asNamespace("quickr"), "path")
    and injects:
    pkgload::load_all(<dev-path>, quiet = TRUE)
    before pkgload::load_all('.') inside the subprocess.

Why this fixes it

The dev quickr emits QuickrEntries with the required sentinel:

static const R_ExternalMethodDef QuickrEntries[] = {
  {"add_ab_", (DL_FUNC) &add_ab_, -1},
  {NULL, NULL, 0}
};

With the sentinel present, R_registerRoutines() stops safely and the DLL loads without crashing.

Verification

  • R -q -e 'devtools::test_active_file("tests/testthat/test-compile-package.R")' passes.
  • Full suite R -q -e 'devtools::test()' passes.
  • R -q -e 'rcmdcheck::rcmdcheck(error_on = "warning")' passes (network warnings only).

Files Changed

  • R/compile-package.R
  • tests/testthat/test-compile-package.R
  • .Rbuildignore (adds ^\.ai$ after air format .)

…t in the tests.

I don't know if something is wrong with my setup or not. But this fixes it on my end.
@mns-nordicals

Copy link
Copy Markdown
Contributor Author

Strange. Now we get the same error that this was suppose to fix. Locally I don't get the error anymore (well - I don't get them on this PR branch and neither on main after running devtools::install() )

So if don't know if it is something with my setup.

Please close/reject if you see no value here.

@t-kalinowski

Copy link
Copy Markdown
Owner

Thanks for pulling this together!

I do think it's tricky to get testing package-related-workflows within a package right. I remember there was a lot of tricky issues around this in S7 as well. It might be worthwhile to look at how we solved that problem there. There we actually run R CMD install during tests on the package being tested, into a temp library.

I don't think that using load_all() is the robust path here.

Ideally, we should be running an actual installation, albeit with some options to make it a a lighter and faster one than normal (the test run time is getting a little too long already).

@mns-nordicals

Copy link
Copy Markdown
Contributor Author

Closing as I didn't run into the problem again and it was fixed by just re-installing the package.

@mns-nordicals
mns-nordicals deleted the package-segfaults branch July 3, 2026 17:38
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.

2 participants