[Rcpp-devel] RCPP_USE_UNWIND_PROTECT by default

[Dirk Eddelbuettel]

Lionel,

Thanks a bunch for the prompt and detailed reply. CCing Jeroen now, I think I
had poked him over DM on this but not followed. Easy for us to delegate to V8 :)
That is of course a complicated (and toolchain-dependent) package too so we
shall see what we can do there.  But the good news on our end is that we can
likely just enable RCPP_UNWIND_PROTECT and everybody (apart from V8) may
indeed be better off.

Cheers, Dirk

On 1 February 2022 at 12:08, Lionel Henry wrote:
| Hi Iñaki,
| 
| IIRC, at the time I was concerned it might be undefined behaviour (from R's
| viewpoint) to catch a longjump exception (wich wraps an R unwind token) without
| rethrowing it. I think this is why I didn't have `LongjumpException` inherit
| from `std::exception` because there seemed to be an assumption that an exception
| inheriting from that class can be arbitrarily caught, and perhaps reported in
| some way to users or callers via the `what()` method.
| 
| A longjump token may represent different kinds of long jumps. If it represents
| an error, it might be reasonable to catch the token and recover. However it can
| also represent a restart invokation, a condition being caught by another handler
| upstack, or a long return (see `base::callCC()`). For these cases the R language
| does not provide a way of interrupting the unwinding. Except, maybe, through
| `on.exit()` expressions on the stack which can throw or long-return, and thus
| overtake the current jump.
| 
| Whether it's UB or not could be checked with Luke. That said, even if well
| defined, I would generally not recommand catching a longjump token and transform
| it to an unknown exception. In general it'd be better to resume the unwinding
| with the token. As mentioned above, it does not necessarily represent an error
| or abnormal situation.
| 
| I'm not familiar with V8 but I took a quick look. IIUC R is called back from JS
| through Rcpp? E.g. via `console.r.call()`? And it looks like throwing exceptions
| (or longjumping) over the JS stack is UB. So I think the proper thing to do is
| to catch `Rcpp::LongjumpException()`, then unwind the JS stack, probably with
| `v8::ThrowException()`, but the longjump exception (or at least the unwind token
| it wraps) needs to be somehow communicated to the other side. Once back in a
| safe Rcpp context with no JS on the stack, resume the longjump with the
| exception or token.
| 
| Of course it's easier to just disable unwind-protection in V8. But implementing
| the approach above would not only improve performance, it would also improve the
| semantics of V8 callbacks to R.
| 
| Best,
| Lionel
| 
| 
| On 1/31/22, Iñaki Ucar <iucar at fedoraproject.org> wrote:
| > Hi Lionel,
| >
| > I've been setting this for years in my own packages, and particularly
| > in a simulator that greatly benefits from this due to its heavy usage
| > of R calls from the C++ simulation loop. Now, we're looking into
| > setting this feature on by default in the next release of Rcpp to
| > improve the performance of other packages, such as httpuv [1], as
| > well.
| >
| > Dirk has run a full revdep check that looks very promising, with just
| > one new failure (the V8 package), and it would be great if you could
| > take a look when you have time. It seems that the UNWIND_PROTECT
| > mechanism somehow collides with v8's exception handling:
| >
| >> test_check("V8")
| >   terminate called after throwing an instance of 'Rcpp::LongjumpException'
| >   Aborted
| >
| > V8 sees this exception, doesn't know what to do with it, and then the
| > program is aborted. This is kind of a Russian Doll, and probably an
| > extreme case, but we are wondering whether we can do something on the
| > Rcpp side to support v8's engine. V8 can always undefine
| > UNWIND_PROTECT if we ship this by default, but this is clearly one of
| > those packages that could benefit from the improved performance that
| > this feature enables.
| >
| > So far, I found that Rcpp::LongjumpException is the only exception in
| > Rcpp that doesn't inherit from std::exception (which is what V8
| > expects). Is this intended/required? I checked that, if
| > LongjumpException inherits from std::exception, V8 doesn't crash there
| > (the exception is recognized), but then two errors are shown in the R
| > console, which is not ideal either:
| >
| >> ctx <- V8::v8()
| >> ctx$eval("console.r.eval('doesnotexists')")
| > Error in eval(ei, envir) : object 'doesnotexists' not found
| > Error: std::exception
| >> x <- try(ctx$eval("console.r.eval('doesnotexists')"))
| > Error : std::exception
| >> x
| > [1] "Error : std::exception\n"
| > attr(,"class")
| > [1] "try-error"
| > attr(,"condition")
| > <std::runtime_error: std::exception>
| >
| > Please let us know if you have some time for this. Any help would be
| > appreciated.
| >
| > Cheers,
| > Iñaki
| >
| > [1] https://github.com/rstudio/httpuv/issues/244
| >
| > --
| > Iñaki Úcar
| >

-- 
https://dirk.eddelbuettel.com | @eddelbuettel | edd at debian.org