Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exception in a later function causes later to stop working #12

Closed
wch opened this issue Sep 19, 2017 · 2 comments · Fixed by #13
Closed

Exception in a later function causes later to stop working #12

wch opened this issue Sep 19, 2017 · 2 comments · Fixed by #13

Comments

@wch
Copy link
Member

wch commented Sep 19, 2017

Example:

library(later)
later(function() {
  cat("1\n", file=stderr())
  stop()
  cat("2\n", file=stderr())
})


later(function() {
  cat("hello\n", file=stderr())
})

This produces the following output when run in the terminal:

> library(later)
> later(function() {
+   cat("1\n", file=stderr())
+   stop()
+   cat("2\n", file=stderr())
+ })
> 1
Error in tryCatch(evalq(sys.calls(), <environment>), error = function (x)  : 
  Evaluation error: .
> later(function() {
+   cat("hello\n", file=stderr())
+ })
> 

Something similar happens if, instead of stop(), you have browser(), and then Q out of the browser -- future calls to later() don't do anything.

@wch
Copy link
Member Author

wch commented Sep 20, 2017

I think the problem is that, when there's an error in the callback function, exec_callbacks_reentrancy_count never gets decremented, so this always returns early:
https://github.com/r-lib/later/blob/1ab43a49/src/later.cpp#L41

It's not getting decremented because the destructor for pcscope isn't executing when the callback throws an error here: https://github.com/r-lib/later/blob/1ab43a49/src/later.cpp#L68

The problem is in the Callback () operator - I think that forward_exception_to_r somehow prevents the destructor from being called: https://github.com/r-lib/later/blob/1ab43a49/src/callback_registry.h#L28-L34

Here's a minimal example that illustrates:

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;
    try {
      func();
    } catch(std::exception &ex) {	
      forward_exception_to_r(ex);
    }
  }
  '
)

invoke(function() {
  cat("R code\n", file=stderr())
})
# MyClass constructor
# R code
# MyClass destructor


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

@wch
Copy link
Member Author

wch commented Sep 20, 2017

I've filed an issue on Rcpp for this: RcppCore/Rcpp#753

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 a pull request may close this issue.

1 participant