Should cpp11::stop just throw an exception rather than using unwind_protect?

[jimhester]

It seems there are some cases where use of unwind_protect causes issues, since there is a long jump involved, such as within constructors.

It might be better to have it simply throw an exception instead.

[jennybc]

I think the motivation for this was probably readxl:

tidyverse/readxl@24a8970

I am revisiting this code, to introduce similar error code checks in another constructor, and have re-established that any natural call to cpp11::stop() will not work. By "natural", I mean simply translating the Rcpp::stop() call.

It reliably leads to:

 *** caught bus error ***
address 0x7fee3c80a0e0, cause 'non-existent physical address'

I'm just noting this in case anyone ever works on this and needs an (non)working example of the problem. I can provide!

[romainfrancois]

If you have such an example, I'll take it :-)

[jennybc]

You can experience the problem by translating this error-throwing code to a simple call to cpp11::stop():

https://github.com/tidyverse/readxl/blob/a019b231ba6d91829b9eca2691d1f8a7735a658a/src/XlsWorkBook.h#L40-L43

Here's a PR where I've induced the problem: tidyverse/readxl#690

I get a segfault (Error: segfault from C stack overflow) from the very first test:

https://github.com/tidyverse/readxl/blob/8d96117e26f2dd25d9fe813bfaf873e46175b61a/tests/testthat/test-coercion.R

Note that there are several other successful uses of cpp11::stop() in constructors in readxl, so the crashing one must be special somehow.

This commit points you at some interesting places to experiment with cpp11::stop() vs. alternatives:

tidyverse/readxl@0cf69c3

[jennybc]

Interestingly, my PR appears to only segfault on macOS. I see it locally and that is the only CI job that fails.

[paleolimbot]

Just a quick +1 for "throwing an exception instead"...in developing user-defined functions (and other "calling into R from other threads" stuff) in Arrow, I ended up with a few cases where the cpp11::unwind_exception thrown by cpp11::stop() was getting caught before it reached CPP11_END. The resulting error message is Error: std::exception, which is rather difficult to debug. (Alternatively, or maybe in addition, one could implement what() for the unwind exception to give a slightly more helpful note along the lines of "this package developer did something very very wrong").

[paleolimbot]

Emulating the printf-like behaviour isn't too bad...this bit of code is mostly untested and was from an early prototype of geoarrow, but might help?

class Exception: public std::exception {
public:
  Exception() {
    memset(error_, 0, sizeof(error_));
  }

  Exception(const std::string& error) {
    memset(error_, 0, sizeof(error_));
    if (error.size() >= (8096 - 1)) {
      memcpy(error_, error.data(), 8096 - 1);
    } else if (error.size() > 0) {
      memcpy(error_, error.data(), error.size());
    }
  }

  Exception(const char* fmt, ...) {
    memset(error_, 0, sizeof(error_));
    va_list args;
    va_start(args, fmt);
    vsnprintf(error_, sizeof(error_) - 1, fmt, args);
    va_end(args);
  }

  const char* what() const noexcept {
    return error_;
  }

protected:
  char error_[8096];
};