Fatal error when loading workspace image with objects containing a null external pointer and a prototype from class definition

Sciurus365 commented 3 years ago

(This issue is forwarded from https://github.com/rstudio/rstudio/issues/8923)

System details

RStudio Edition : Desktop
RStudio Version : 1.4.1103
OS Version      : Win10
R Version       :  4.0.3

Steps to reproduce the problem

library(bigmemory)
setClass("testClass1",
                 slots = c(md5 = "character"),
                 contains = "big.matrix")

setClass("testClass2",
                 slots = c(md5 = "character"),
                 prototype = c(md5 = "1"),
                 contains = "big.matrix")

t1 <- as.big.matrix(matrix(1:100, nrow = 10))

t2 <- new("testClass1", address = t1@address, md5 = digest::digest(matrix(1:100, nrow = 10)))

t3 <- new("testClass2", address = t1@address, md5 = digest::digest(matrix(1:100, nrow = 10)))

Describe the problem in detail

If I run the codes above, save the workspace, restart RStudio again, it will throw a fatal error. This only occurs if t3 is in the workspace, and only occurs in RStudio (RGui doesn't have this problem). I guess a part of this problem comes from the behavior of bigmemory package: it generates external pointers which will become null in the next time. If I click t2 with a null pointer to view it, the fatal error of R will also occur. (This is easy to avoid: as long as I don't click it, it's fine.) But for t3, whenever I load the workspace, that fatal error occurs.

Another thing I found is that t2 is in the Data section of the variable inspector, while t3 is in the Values section. I suspect this is related to this problem, but I'm not sure.

Describe the behavior you expected

Successfully load the abovementioned objects.

[x] I have read the guide for submitting good bug reports.
[x] I have installed the latest version of RStudio, and confirmed that the issue still persists.
[x] If I am reporting a RStudio crash, I have included a diagnostics report.
[x] I have done my best to include a minimal, self-contained set of instructions for consistently reproducing the issue.

Sciurus365 commented 3 years ago

I believe this is a bug that should be fixed in the bigmemory package. You can reproduce something similar in plain R, with:

library(bigmemory)
t1 <- as.big.matrix(matrix(1:100, nrow = 10))
saveRDS(t1, file = "bigmem.rds")
t1 <- readRDS("bigmem.rds")
str(t1)

The call to str() segfaults, because bigmemory fails to check for null external pointers. We could (and perhaps should) work around this by avoiding calls to str() on such objects. My suspicion is that this is happening here:

https://github.com/rstudio/rstudio/blob/4d1fc87218497d67342f961e27eeb151e8d161a9/src/cpp/session/modules/SessionEnvironment.R#L45-L51

Originally posted by @kevinushey in https://github.com/rstudio/rstudio/issues/8923#issuecomment-876814237

privefl commented 3 years ago

This is an old issue with external pointers, and is not specific to str() (you would get a crash if e.g. trying to get t1[1] as well). The solution used in {bigmemory} is to work with descriptors. A more elegant solution is to use a RC object with active bindings for the external pointer, e.g. as implemented in package {bigmemoryExtras}.

privefl commented 3 years ago

This is discussed a bit in https://github.com/kaneplusplus/bigmemory/issues/100.

Sciurus365 commented 3 years ago

Thank you @privefl ! I'll check that out. By the way, I saw https://www.bioconductor.org/packages/release/bioc/html/bigmemoryExtras.html that the package bigmemoryExtras is deprecated. Is that the case? Do you know its current status or possible alternatives?

privefl commented 3 years ago

I don't know about the current status of {bigmemoryExtras}; @phaverty?

I implement something similar to big.matrix objects in my package {bigstatsr}; see similarities and differences at https://privefl.github.io/bigstatsr/articles/bigstatsr-and-bigmemory.html.

phaverty commented 3 years ago

Hi Florian,

Thanks for your interest. bigmemoryExtras has recently fallen into disrepair. It could probably be resurrected with some small updates.

On Tue, Sep 21, 2021 at 8:37 AM Florian Privé @.***> wrote:

I don't know about the current status of {bigmemoryExtras}; @phaverty https://github.com/phaverty?

I implement something similar to big.matrix objects in my package {bigstatsr}; see similarities and differences at https://privefl.github.io/bigstatsr/articles/bigstatsr-and-bigmemory.html.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kaneplusplus/bigmemory/issues/106#issuecomment-924108876, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB6TMK7VUVKXGKBMUCKEFO3UDCRFHANCNFSM5EOACMJQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

Sciurus365 commented 3 years ago

@privefl Thanks! I'll check it out.

kaneplusplus / bigmemory