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

Should use R_TopLevelExec when executing callbacks #91

Closed
wch opened this issue Apr 17, 2019 · 1 comment
Closed

Should use R_TopLevelExec when executing callbacks #91

wch opened this issue Apr 17, 2019 · 1 comment

Comments

@wch
Copy link
Member

wch commented Apr 17, 2019

Currently, callbacks that are executed by run_now can affect the caller, but we don't want this.

My understanding is that if we execute the callbacks with R_TopLevelExec, it acts as a barrier which exceptions and returns won't cross.

Here's an example that demostrates the return issue (adapted from callCC) where an early return affects the run_now, and prevents subsequent callbacks from being executed:

f <- function() {
  delayedAssign("throw", return(100))
  
  later::later(function() {
    throw
  }, 2)
  
  later::later(function() message("hello"), 2)
  
  later::run_now(3)
  200
}

f()
#> [1] 100

The f() returns 100, but that's not what we'd expect. Also, the subsequent callback with message("hello") just doesn't execute. For some reason, after running this code, the callbacks no longer run at an idle console. I think this is probably related to the early return. Calling run_now() again does execute the message("hello"), however.

Although the example above uses an unusual case (an early return) I believe a similar thing happens with exceptions, and I think that the R_TopLevelExec will prevent these problems.

@wch
Copy link
Member Author

wch commented Aug 21, 2019

This was implemented as part of #84.

@wch wch closed this as completed Aug 21, 2019
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

No branches or pull requests

1 participant