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

Finalize method for cloned objects is executed in the wrong environment #167

Closed
s-fleck opened this issue Nov 11, 2018 · 2 comments

Comments

@s-fleck
Copy link

commented Nov 11, 2018

When cloning an R6 object, it seems the finalize method still refers to the wrong Object. The code below should illustrate the problem quite well:

happy <- R6::R6Class(
  public = list(
    foo = NULL,
    initialize = function() {self$foo <- ":("},
    happyify   = function() {self$foo <- ":)"},
    finalize   = function() {cat("foo: ", self$foo, "\n")}
  )
)

x <- happy$new()
y <- x$clone()

y$happyify()
y$.__enclos_env__$self$foo

#> [1] ":)"

rm(y)
gc()

#> foo:  :( 

This could be related to #155 but I tried it with the newest dev version from github, and the problem still persisted :(

workarounds would also be welcome.

@wch

This comment has been minimized.

Copy link
Member

commented Feb 14, 2019

It looks like y's finalizer actually isn't being run at all. The foo :( you're seeing is from something else -- I think it's probably left over from previous runs. You can see this better if you start off by clearing everything, or in a new R session:

# Start off by removing everything
rm(list = ls(all.names = TRUE))
gc()

happy <- R6::R6Class(
  public = list(
    foo = NULL,
    initialize = function() {self$foo <- ":("},
    happyify   = function() {self$foo <- ":)"},
    finalize   = function() {cat("foo: ", self$foo, "\n")}
  )
)
x <- happy$new()
y <- x$clone()
y$happyify()
rm(y)
gc()

rm(x)
gc()
#>  foo:  :(

The reason that y's finalizer isn't being run is because the clone method currently doesn't register the finalizer, whereas the new method does:

R6/R/new.R

Lines 151 to 168 in 748bb60

# Finalize --------------------------------------------------------
if (is.function(.subset2(public_bind_env, "finalize"))) {
# This wraps the user's `finalize` method. The user's finalize method
# typically does not have an `e` argument, so the wrapper needs to consume
# the `e` argument.
finalizer_wrapper <- function(e) {
.subset2(e, "finalize")()
}
# Reassign the wrapper's environment so that it does not capture the current
# environment and prevent objects from getting GC'd.
environment(finalizer_wrapper) <- baseenv()
reg.finalizer(
public_bind_env,
finalizer_wrapper,
onexit = TRUE
)
}

@wch

This comment has been minimized.

Copy link
Member

commented Feb 14, 2019

Fixed by #180.

@wch wch closed this Feb 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.