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

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

Open
4 tasks done
Sciurus365 opened this issue Sep 21, 2021 · 7 comments

Comments

@Sciurus365
Copy link

(This issue is forwarded from rstudio/rstudio#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.

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

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 rstudio/rstudio#8923 (comment)

@privefl
Copy link
Contributor

privefl commented Sep 21, 2021

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
Copy link
Contributor

privefl commented Sep 21, 2021

This is discussed a bit in #100.

@Sciurus365
Copy link
Author

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
Copy link
Contributor

privefl commented Sep 21, 2021

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
Copy link
Contributor

phaverty commented Sep 21, 2021 via email

@Sciurus365
Copy link
Author

@privefl Thanks! I'll check it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants