r-lib / R6

Encapsulated object-oriented programming for R
https://R6.r-lib.org
Other
405 stars 56 forks source link

R6 constructor creating persistent reference to arguments #121

Closed richfitz closed 7 years ago

richfitz commented 7 years ago

I'm not sure this is exactly a bug, but this is behaviour that surprised me. When passing an object through the constructor of an R6 object, the object passed as an argument will not be garbage collected until the newly created object is garbage collected.

Here is a minimal example:

A <- R6::R6Class(
  "A",
  cloneable = FALSE,
  public = list(
    value = NULL,
    initialize = function(value) {
      self$value <- value
    },
    finalize = function() {
      message("Deleting object A with value ", self$value)
    }))

B <- R6::R6Class(
  "B",
  cloneable = FALSE,
  public = list(
    initialize = function(a) {
      message(sprintf("I was passed %s in constructor", a$value))
    },
    finalize = function() {
      message("Deleting object B")
    }))

Create a copy of A and pass that through to B which just ignores the argument

a <- A$new(1)
b <- B$new(a)

Deleting a should (to my intuition anyway) leave no references to a but the finalizer is not called

rm(a)
gc()

...until b is deleted and garbage collected:

rm(b)
gc()
## Deleting object B
## Deleting object A with value 1

I've poked about but I can't work out what environment a reference to A is being kept. This works the same way with cloneable set to TRUE or FALSE, and the behaviour is the same for non-portable classes

X <- R6::R6Class(
  "X",
  cloneable = FALSE,
  portable = FALSE,
  public = list(
    value = NULL,
    initialize = function(value) {
      value <<- value
    },
    finalize = function() {
      message("Deleting object X with value ", value)
    }))

Y <- R6::R6Class(
  "Y",
  cloneable = FALSE,
  portable = FALSE,
  public = list(
    initialize = function(a) {
      message(sprintf("I was passed %s in constructor", a$value))
    },
    finalize = function() {
      message("Deleting object Y")
    }))

x <- X$new(1)
y <- Y$new(x)
rm(x)
gc()
rm(y)
gc()
## Deleting object Y
## Deleting object X with value 1

In the end, nothing is terrible - things get deleted eventually. But it's a bit of a problem for a DB wrapper I am writing as it's overly conservative about when things can be deleted.

wch commented 7 years ago

Thanks for finding this bug. I think this is happening because the finalizer is capturing the parent environment. Here's a modified example that uses R's reg.finalizer instead of an R6 finalize method:

library(R6)
A <- R6::R6Class(
  "A",
  public = list(
    value = NULL,
    initialize = function(value) {
      self$value <- value
    }
  )
)

B <- R6::R6Class(
  "B",
  public = list(
    initialize = function(a) {
      message(sprintf("I was passed %s in constructor", a$value))
    }
  )
)

a <- A$new(1)
reg.finalizer(a, function(e) message("Deleting object A"))

b <- B$new(a)
reg.finalizer(b, function(e) message("Deleting object B"))

rm(a)
gc()
# Deleting object A

rm(b)
gc()
# Deleting object B

Here's the part where the finalizer captures the parent environment. It happens because the finalize method gets wrapped in another function: https://github.com/wch/R6/blob/2ce4882/R/new.R#L153-L157

The fix should be pretty straightforward.

richfitz commented 7 years ago

Thanks - this is great. I can confirm things working as expected with the update, too :)