`object.size()` call crashes with `np` object created from `example(R6)`

[kevinushey]

Repro:

library(R6)
example(R6Class)
object.size(np)

gives a segfault from C stack overflow. Likely due to this being a self-referential object?

This is a problem for the IDE as we call object.size() when building the environment pane and so running the R6 examples crashes RStudio.

[wch]

Hm, self reference in and of itself doesn't seem to cause R to crash. For example:

e <- new.env()
e$e <- e
object.size(e)
# 56 bytes

FWIW, pryr's version of object_size works, and @hadley says it's more accurate than object.size.

> pryr::object_size(np)
79.2 kB

[kevinushey]

I'll see if I can get a MRE without using R6.

[wch]

Hm, it looks like the problem only happens when portable=FALSE.

I think I've tracked down the problem - it's when an attribute refers to the original object. For example:

# This is OK
e <- new.env()
e$e <- e
object.size(e)
# 56 bytes

# Using attributes causes segfault
e <- new.env()
attr(e, "e") <- e
object.size(e)
# Segmentation fault

[kevinushey]

That was fast! Should we make a bug report / post on R-devel?

[wch]

Sure, I can do it. Will you be able to work around this in RStudio? I believe the only attribute you need to watch out for in R6 objects is enclos_env.

[kevinushey]

Yep, it should be possible to work around in RStudio.

[wch]

Filed here: https://bugs.r-project.org/bugzilla3/show_bug.cgi?id=16441

[hadley]

If we really want to avoid bugs like this in RStudio, I think the IDE will need to switch to using lobstr...

[wch]

@kevinushey Can we close this issue now?

[kevinushey]

Yes, I think so.

[kevinushey]

Sorry, I should add one more thing -- calling str(np) itself also causes a recursion overflow; should R6 add a str.R6 method to avoid this?

[hadley]

If you want to use a buggy function (i.e. str()), I think you have to live with the consequences (or report bugs upstream and hope they get fixed).

[wch]

@kevinushey Good point. Although, even if I add a str.R6 method, this could still result in crashes in the case where someone uses class=FALSE (where the environment doesn't have any classes attached to it). I'll see if I can do a different workaround so that the problem doesn't come up at all.

[wch]

@kevinushey I decided to remove the attribute, because in the bug I filed, Luke Tierney said this: "Attributes on environments are always a bad idea. No matter how convenient they may seem, the implementation doesn't support them in a reasonable way."

So if you were thinking of adding a workaround to RStudio, it's not necessary anymore.