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

Returning invisible(self) in print method prevents the object to be gc'ed #140

Closed
Enchufa2 opened this issue Mar 26, 2018 · 10 comments

Comments

@Enchufa2
Copy link

commented Mar 26, 2018

I'm not sure about what's going on here. This works ok:

library(R6)

Person <- R6Class("Person",
  public = list(
    initialize = function() {
      self
    },
    print = function() {
      cat(paste0("Hello, world!\n"))
      # invisible(self)
    },
    finalize = function() {
      message("Finalizer has been called!")
    }
  )
)
Person$new()
gc() # still in .Last.value
gc() # Finalizer has been called!

The object is gc'ed in the second call as expected, because in the first call, the object is still accessible through .Last.value. But if we return invisible(self) from the print method, as #125 says and as every print method in R does, the object is not gc'ed:

library(R6)

Person <- R6Class("Person",
  public = list(
    initialize = function() {
      self
    },
    print = function() {
      cat(paste0("Hello, world!\n"))
      invisible(self)
    },
    finalize = function() {
      message("Finalizer has been called!")
    }
  )
)
Person$new()
gc() # still in .Last.value
gc() # nothing...

But if we print something else, then the object is gc'ed!

print(1)
gc() # Finalizer has been called!
@wch

This comment has been minimized.

Copy link
Member

commented Mar 26, 2018

I suspect the root cause is that R is doing some sort of caching for printed objects. I'm not sure whether this is a bug in R, or if there's a good reason for it.

The same thing happens with this code:

print.foo <- function(x, ...) {
  cat("print.foo called\n")
  invisible(x)
}

new_foo <- function() {
  e <- new.env()
  reg.finalizer(e, function(e) message("Finalizer called"))
  class(e) <- "foo"
  e
}

new_foo()
gc() # still in .Last.value
gc() # nothing

If the last line of new_foo() is changed from e to invisible(e), then it does run the finalizer on the second gc().

@wch

This comment has been minimized.

Copy link
Member

commented Mar 26, 2018

It might actually be due to caching in R's S3 dispatch mechanism. For example, this executes the finalizer:

ident <- function(x) invisible(x)

new_foo <- function() {
  e <- new.env()
  reg.finalizer(e, function(e) message("Finalizer called"))
  class(e) <- "foo"
  e
}

ident(new_foo())
gc()
gc() # Finalizer called

But this doesn't:

ident <- function(x) UseMethod("ident")
ident.foo <- function(x) invisible(x)

new_foo <- function() {
  e <- new.env()
  reg.finalizer(e, function(e) message("Finalizer called"))
  class(e) <- "foo"
  e
}

ident(new_foo())
gc()
gc() # Nothing

After running the previous block of code, any of these 3 code snippets will trigger the finalizer.

# Calling `ident` generic on another object:
ident.default <- function(x) invisible(x)
ident(1); gc()


# Calling `print` on another object:
print(1); gc()


# Calling a completely different S3 generic on another object:
abc <- function(x) UseMethod("abc")
abc.default <- function(x) invisible(1)
abc(1); gc()

Calling another non-S3 function before the gc() does not work:

identity(1); gc()  # Nothing
sum(1); gc()       # Nothing
invisible(1); gc() # Nothing
@Enchufa2

This comment has been minimized.

Copy link
Author

commented Mar 26, 2018

You are right, nice analysis!

So, is this a bug or a feature? Do you think I should report this in R-devel?

@Enchufa2

This comment has been minimized.

Copy link
Author

commented Mar 26, 2018

BTW, closing this, as it has nothing to do with R6.

@Enchufa2 Enchufa2 closed this Mar 26, 2018

Enchufa2 added a commit to r-simmer/simmer that referenced this issue Mar 26, 2018
@wch

This comment has been minimized.

Copy link
Member

commented Mar 26, 2018

It might be worth reporting to r-devel, or at least asking if it's expected behavior.

@Enchufa2

This comment has been minimized.

Copy link
Author

commented Mar 26, 2018

I was about to post on R-devel and...

Did you try your examples (and mine) in a plain R console? I think it's RStudio...

@gaborcsardi

This comment has been minimized.

Copy link
Member

commented Mar 26, 2018

For me it fails in the terminal as well, but only if you run it in a fresh R session.

@Enchufa2

This comment has been minimized.

Copy link
Author

commented Mar 26, 2018

Ok, I'm trying to gather the final bits. It was R 3.2.3 (don't ask...) the one that succeeded. R 3.4.3 fails miserably with or without RStudio. It's R after all. Someone with quick access to R 3.3.x?

@Enchufa2

This comment has been minimized.

Copy link
Author

commented Mar 26, 2018

Checked. It fails on R 3.3.2 too. Writing to R-devel...

@Enchufa2

This comment has been minimized.

Copy link
Author

commented Mar 27, 2018

For the sake of completeness, if somebody lands here, Luke Tierney explains the issue here. It turns out that it has nothing to do with S3 dispatch, but with an internal register being protected as a result of a explicit return. The following example shows the same issue without S3:

new_foo <- function() {
  e <- new.env()
  reg.finalizer(e, function(e) message("Finalizer called"))
  e
}

bar <- function(x) return(x)

bar(new_foo())
gc() # still in .Last.value
gc() # nothing

The internal register gets cleared as soon as there is another explicit return:

bar(1)
gc() # Finalizer called

Note that the important bit is the explicit return. If we remove it, the issue disappears:

new_foo <- function() {
  e <- new.env()
  reg.finalizer(e, function(e) message("Finalizer called"))
  e
}

bar <- function(x) x

bar(new_foo())
gc() # still in .Last.value
gc() # Finalizer called!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.