Crash caused by some interaction between testthat/V8

[richfitz]

This one has me baffled, I've spent a couple of hours narrowing it down, so hopefully something will jump out at you!

The repository https://github.com/richfitz/crash contains a trivial package using V8 that reliably causes a crash when running testthat::test_local

Prepare by running, from the clone of richfitz/crash

docker pull wch1/r-debug
docker run -v $PWD:/src:ro -it --rm \
       --security-opt seccomp=unconfined \
       wch1/r-debug
apt-get update && apt-get install -y libv8-dev
RDcsan -e 'install.packages(c("testthat", "V8"))'

This installs the CRAN version of V8 (3.4.2) from source, which on this container ends up with:

> RDcsan -e 'V8::engine_info()'
...
$version
[1] "6.8.275.32-node.55"

Running the tests (which simply load V8::v8() within a test_that block, crashes R:

RDcsan -e 'testthat::test_local("/src")'

Top part of the call stack:

✔ | F W S  OK | Context
⠏ |         0 | crash
Exception thrown during bootstrapping
/usr/include/v8/v8.h:8921:17: runtime error: member call on null pointer of type 'v8::Context'
    #0 0x7fa2a092e0a1 in v8::Context::Scope::Scope(v8::Local<v8::Context>) /usr/include/v8/v8.h:8921:17
    #1 0x7fa2a092e0a1 in make_context(bool) /tmp/RtmpwYeYmx/R.INSTALL10e91936780e/V8/src/bindings.cpp:314:22
    #2 0x7fa2a08ea4c6 in _V8_make_context /tmp/RtmpwYeYmx/R.INSTALL10e91936780e/V8/src/RcppExports.cpp:75:34
    #3 0x7fa2b666c766 in R_doDotCall /tmp/r-source/src/main/dotcode.c:601:17

This, outside of testthat, does not crash though:

RDcsan -e 'V8::v8()'

A more interesting example, after starting with RDcsan

test_path <- "/src/tests/testthat"
env <- pkgload::load_all(test_path)$env
setwd(test_path)
# This is fine:
testthat:::source_file("test-crash.R", rlang:::child_env(env), wrap = FALSE)
# This crashes
testthat:::source_file("test-crash.R", rlang:::child_env(env), wrap = TRUE)

I saw this issue (r-lib/pkgload#96) and tested with an older pkgload (CRAN release 1.2.1, predating that PR) with no effect:

Possibly related to this V8 issue: #119 / rstudio/shiny#3289 (comment)

Following this comment, I tried #119 (comment)

RDcsan -e "Sys.setenv(DOWNLOAD_STATIC_LIBV8 = 1); \
    install.packages('V8', repos = 'https://cran.r-project.org')"

which gives list(version = "8.3.110.13") from V8::engine_info()

This does produce output from the sanitiser during install and every use of the package:

/usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/shared_ptr_base.h:155:46: runtime error: member access within address 0x613000099780 which does not point to an object of type 'std::_Sp_counted_base<__gnu_cxx::_S_atomic>'
0x613000099780: note: object has invalid vptr
 00 00 00 00  28 85 4a ad 06 7f 00 00  05 00 00 00 01 00 00 00  38 50 4c ad 06 7f 00 00  00 be be be
              ^~~~~~~~~~~~~~~~~~~~~~~
              invalid vptr
    #0 0x7f06ac84d78e in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/shared_ptr_base.h:155:46
    #1 0x7f06ac88a76a in v8::Isolate::Initialize(v8::Isolate*, v8::Isolate::CreateParams const&) (/usr/local/RDcsan/lib/R/site-library/00LOCK-V8/00new/V8/libs/V8.so+0x5a076a)
    #2 0x7f06ac88a79c in v8::Isolate::New(v8::Isolate::CreateParams const&) (/usr/local/RDcsan/lib/R/site-library/00LOCK-V8/00new/V8/libs/V8.so+0x5a079c)
    #3 0x7f06ac82e279 in start_v8_isolate(void*) /tmp/Rtmp4vStcO/R.INSTALLf2b70824c9e/V8/src/bindings.cpp:62:13
    #4 0x7f06ac805e42 in R_init_V8 /tmp/Rtmp4vStcO/R.INSTALLf2b70824c9e/V8/src/RcppExports.cpp:106:5

and a different error for the test

RDcsan  -e 'testthat::test_local("/src")'
[ ... UB error as above ]
✔ | F W S  OK | Context
✖ | 1       0 | crash [0.3s]                                                    
────────────────────────────────────────────────────────────────────────────────
Error (test-crash.R:2:3): a simple test
<std::runtime_error/C++Error/error/condition>
Error in `context_eval(paste("var", global, "= this;", collapse = "\n"), 
    private$context)`: <string conversion failed>
Backtrace:
 1. V8::v8() test-crash.R:2:2
 2. (function() {...
 3. V8:::reset()
 4. V8:::context_eval(...)
────────────────────────────────────────────────────────────────────────────────

══ Results ═════════════════════════════════════════════════════════════════════
Duration: 0.7 s

[ FAIL 1 | WARN 0 | SKIP 0 | PASS 0 ]

This is possibly the same issue as BDR gets on fedora-clang-devel (https://www.r-project.org/nosvn/R.check/r-devel-linux-x86_64-fedora-clang/jsonvalidate-00check.html) though that could be it's own weird thing of course. It's very odd that this does not trigger on any of the V8 tests. My guess is that it might be different pulling V8 in vs testing it so a package that uses V8 and has compiled code may get things showing up on CRAN's additional issues page. One additional weirdness is that the packages that were using jsonvalidate are not showing similar errors (e.g., https://cran.r-project.org/web/checks/check_results_biocompute.html, which does not error on r-devel-linux-x86_64-fedora-clang, even though it does run a test using jsonvalidate/V8)

I'm also guessing that testthat is sort of a red herring here, and the difference is the call stack depth?

[JanMarvin]

You can run the tests like this. Iirc the stacktrace is caused by the load_package option of testthat::test_dir(). R and V8 simply do not like this behavior and there is nothing wrong. My best guess is that this should motivate a switch to e.g. tinytest.

testlocal <- function (path = ".", reporter = NULL, ...)
{
  package <- pkgload::pkg_name(path)
  test_path <- file.path(pkgload::pkg_path(path), "tests", "testthat")
  withr::local_envvar(NOT_CRAN = "true")
  testthat::test_dir(test_path, package = package, reporter = reporter,
                     ..., load_package = "none")
}

testlocal()

[richfitz]

Thanks for the comment - however, I am fairly sure this is a problem and I disagree that moving to tinytest is a reasonable fix for this.

To clarify the ultimate problem - the above was trying to isolate a problem that I am seeing with jsonvalidate (ropensci/jsonvalidate#54) trying to replicate a crash seen on BDR's machine.

Closer to the way that CRAN runs tests still crashes (though seemingly not with the simple package)

docker pull wch1/r-debug
docker run -v $PWD:/src:ro -it --rm \
       --security-opt seccomp=unconfined \
       wch1/r-debug
apt-get update && apt-get install -y git libv8-dev
git clone --single-branch --branch=i51-cran-update https://github.com/ropensci/jsonvalidate
RDcsan -e 'install.packages(c("testthat", "V8"))'
RDcsan CMD INSTALL /src/jsonvalidate --install-tests
RDcsan -e '(tools::testInstalledPackage("jsonvalidate"))' # code 1 is error
cat jsonvalidate-tests/testthat.Rout.fail # same trace as before

[jeroen]

There are a few separate issues here:

1. Using testthat/devtools with pkgload::load_all without a proper installation simply does not work for V8. We need to initiate / teardown global state for libv8 on the C++ level, which does not happen when pkgload tries to hotswap the pkg dll file. We need to do a proper installation of V8 to run the tests.

2. When running V8 in RDcsan we were hitting stack limits, which would cause libv8 to fail to load javascript snippets, with a mysterious error <string conversion failed>. I have now disabled the stack limit when running sanitizers and also tried to give better error messages in case of unexpected load failures which indicate we are hitting stack limits.

3. This still does not explain the double-free error we were seeing on fedora-clang, but I suspect that this is a problem in BDR's setup. The same problem happend when running tests of the V8 package itself, which BDR has stop-listed for fedora-clang. Hopefully it will disappear once they upgrade their Fedora with a more recent libv8.

[richfitz]

I don't think any of the bits above ran load_all on V8 (or did not mean to in any case), just the package under test. Otherwise that all looks about right unfortunately. I guess that means it's not totally safe for any package with V8 as a dependency to run tests on CRAN though?

[jeroen]

I haven't seen any problems with packages using the regular check_package() during CMD check. But I think testthat::test_local uses the pkgload package which does not always work well.

But I think at least most of your examples above should be fixed now that I removed the stacklimits when running with sanitizers.

[glin]

I just had to fix a really tough double free crash on CRAN's fedora-clang platform with my V8-using package too. Maybe it's the same issue, or related, or at least helpful in some way.

For whatever reason, the double free error was being caused by an uncaught exception in V8 there. To reproduce, you can use R-hub's fedora-clang-devel image (Fedora 33 and a bit behind CRAN, but still works). Install V8 using g++, not clang, as CRAN does, and then throw any error in V8 like throw new Error().

docker run -it --rm rhub/fedora-clang-devel

dnf install -y v8-devel

# Compile V8 with g++ as CRAN does - there's probably a better way to do this
cp ~/.R/Makevars ~/.R/Makevars.bk
echo 'CXX17=g++' >> ~/.R/Makevars
/opt/R-devel/bin/R -e 'options(repos = "https://cloud.r-project.org"); install.packages("V8")'
mv -f ~/.R/Makevars.bk ~/.R/Makevars

/opt/R-devel/bin/R -e 'ctx <- V8::new_context(); ctx$eval("throw new Error()")'
# > ctx <- V8::new_context(); ctx$eval("throw new Error()")
# free(): double free detected in tcache 2
# Aborted (core dumped)

My package was throwing an error to detect the broken V8 package on Fedora <= 36 (#65) to skip some tests, and hitting this crash. My sketchy workaround was to try and test for this in a separate R process like:

code <- "V8::new_context()$eval('not_defined')"
output <- suppressWarnings(
  system2(R.home("bin/R"), c("-e", shQuote(code)), stdout = TRUE, stderr = TRUE)
)
if (attr(output, "status") > 0 && !grepl("ReferenceError", paste(output, collapse = "\n"))) {
  # Then skip any tests that may throw uncaught exceptions
}

[jeroen]

@glin thanks for the reproducer. I think this is a separate issue that is caused by an ABI incompatibility between libcxx (used by R) and libstdc++ (used by Fedora to compile v8-devel).

Can you open a new issue for this?

[glin]

@jeroen Thanks, and done! just opened #161.