When calling a Function, destructors do not execute when quitting from debugger

[wch]

This is a follow-up to #753. @jjallaire said this might not be fixable but I'm filing it here for future reference.

Destructors do not execute when the C++ code calls a Function, the function goes into the R debugger console, and the user quits with Q.

Rcpp::cppFunction(
  includes = '
  class MyClass {
  public:
    MyClass() {
      fprintf(stderr, "MyClass constructor\\n");
    }
    ~MyClass() {
      fprintf(stderr, "MyClass destructor\\n");
    }
  };
  ',
  
  code = '
  void invoke(Function func) {
    MyClass m;
    func();
  }
  '
)

# With stop(), destructor works.
invoke(function() {
  stop()
  cat("R code\n", file=stderr())
})
# MyClass constructor
# MyClass destructor
# Error in invoke(function() { : Evaluation error: .

# When in browser, press Q to quit.
# Destructor does not run.
invoke(function() {
  browser()
  cat("R code\n", file=stderr())
})
#> MyClass constructor
#> Called from: (function () 
#> {
#>     browser()
#>     cat("R code\\n", file = stderr())
#> })()
# Browse[1]> Q

[lionel-]

I am experimenting with a new approach for handling longjumps. You can find it at lionel-/r-source@62d1d81.

It does not require wrapping the R code in a tryCatch(), which should improve performance, and it handles any sort of longjumps: restarts, long scoped return(), exit handling of conditions of any class, unhandled interrupt, error or warning converted to error, Q in browser(), etc.

The idea is to declare a pair of forwarded and forwarding contexts.

1. The Rcpp function captures the context pointer of the R closure that wraps .Call(). This is done with a new function R_GetContextHandle() that returns a pointer.

2. Running R code is done with R_tryEvalForward(). This creates a forwarding context and takes a pointer to the forwarded frame.

3. When a longjump occurs and we find a forwarding frame on the stack, the jumptarget is set to the actual target and we jump to the forwarding frame.

4. At that point R_ForwardExec() (which powers R_tryEvalForward()) sets the jumptarget of the forwarded frame. This means that endcontext() will continue the longjump to the actual target once the Rcpp wrapper returns.

Effectively the longjump is interrupted between the forwarding and forwarded frame which leaves Rcpp free to unwind the stack properly.

I still need to test this more thoroughly and double check everything but the preliminary tests work well.

[eddelbuettel]

Let's get Rcpp 0.12.13 out of the way in a few days, and then we have plenty of time to test this.

Much appreciate you looking into this.

[kevinushey]

I recall we switched to tryCatch() as opposed to R_TopLevelExec() to ensure that the following kinds of calls worked:

withCallingHandlers(
    rcpp_warning(),
    warning = function(w) {
        print("Hello from warning handler!")
    }
)

or:

tryCatch(
    rcpp_warning(),
    warning = function(w) {
        print("Hello from warning handler!")
    }
)

where rcpp_warning() is an Rcpp-exported R function which itself emits an R warning. Does this work as expected with your scheme? (It seems like it would but want to double-check)

[lionel-]

Yes it works well. I still need to think through what happens when another longjump occurs between the forwarded and forwarding frames (these may need better names).

[lionel-]

Ok the patch is ready I think, I'm going to send it to Luke and Tomas: https://github.com/lionel-/r-source/pull/3

Here is how Rcpp would use the new facilities: master...lionel-:impl-unwind

I wonder if LongjumpException should be in the Rcpp namespace rather than in Rcpp::internal. This way Rcpp users could exclude the longjump exception from catch-all statements, since it really needs to be thrown all the way to the wrapper macros.

[jjallaire]

Cool!!! We'd probably need to conditionally compile this codepath so that it works in newer versions of R that have the patch but remains compatible with older versions. Hopefully there is an #ifdef check suitable for this available.

…

On Thu, Sep 28, 2017 at 10:39 AM, Lionel Henry ***@***.***> wrote: Ok the patch is ready I think, I'm going to send it to Luke and Tomas: lionel-/r-source#3 <https://github.com/lionel-/r-source/pull/3> Here is how Rcpp would use the new facilities: master...lionel-:impl-unwind <master...lionel-:impl-unwind> I wonder if LongjumpException should be in the Rcpp namespace rather than in Rcpp::internal. This way Rcpp users could exclude the longjump exception from catch-all statements, since it really needs to be thrown all the way to the wrapper macros. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#754 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAGXx19oONEB0clFi5jsxERcpEDMvgYjks5sm6--gaJpZM4PefNV> .

[lionel-]

Yes we can use this:

#include <Rversion.h>

#if (defined(R_VERSION) && R_VERSION > R_Version(3, 5, 0))

We can make Rcpp_fast_eval() an alias of Rcpp_eval() on old R versions.

[eddelbuettel]

You rock! We just got Rcpp 0.12.13 onto CRAN, always takes a few days from our end and then Uwe was traveling earlier in the week too -- so I was about to poke you.

Now, and I'm just out of meetings so apologies if you detail this somewhere: how could we / would we use this before R 3.5.0 is out? Ie can R-release and Rcpp access this?

[lionel-]

If the patch is accepted we can integrate it with Rcpp with the conditional codepaths as suggested by JJ. Then the new stack-unwinding system will be enabled on R-devel distributions, including on Travis. We'll first have to see if Luke finds the approach sound and accepts the patch. Fingers crossed!

[kevinushey]

Could Rcpp_fast_eval() (or some alternate built on top of that) work as a substitute for Rcpp_eval() itself? It'd be awesome if we could avoid the tryCatch() overhead.

[lionel-]

It could but I was afraid some code might be relying on Rcpp converting R errors to C++ exceptions. It seemed safer to preserve the current behaviour and let packages progressively move to the faster eval function if they need the performance.

[lionel-]

Luke said he's not happy with this approach and that he and Tomas will try to come up with something for the next release if they have time.

[lionel-]

Luke committed the unwind API a few days ago! I'm working on a PR to use it in Rcpp.

wch/r-source@c8cc608

[github-actions]

This issue is stale (365 days without activity) and will be closed in 31 days unless new activity is seen. Please feel free to re-open it is still a concern, possibly with additional data.